-
Notifications
You must be signed in to change notification settings - Fork 2
Dialect: Add support for asyncpg and psycopg3 drivers
#11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
20ab16e to
e1929cb
Compare
tests/engine_test.py
Outdated
| def test_engine_sync_vanilla(): | ||
| """ | ||
| crate:// -- Verify connectivity and data transport with vanilla HTTP-based driver. | ||
| """ | ||
| engine = sa.create_engine("crate://crate@localhost:4200/", echo=True) | ||
| assert isinstance(engine, sa.engine.Engine) | ||
| with engine.connect() as connection: | ||
| result = connection.execute(QUERY) | ||
| assert result.mappings().fetchone() == {'mountain': 'Acherkogel', 'coordinates': [10.95667, 47.18917]} | ||
|
|
||
|
|
||
| def test_engine_sync_urllib3(): | ||
| """ | ||
| crate+urllib3:// -- Verify connectivity and data transport *explicitly* selecting the HTTP driver. | ||
| """ | ||
| engine = sa.create_engine("crate+urllib3://crate@localhost:4200/", isolation_level="AUTOCOMMIT", echo=True) | ||
| assert isinstance(engine, sa.engine.Engine) | ||
| with engine.connect() as connection: | ||
| result = connection.execute(QUERY) | ||
| assert result.mappings().fetchone() == {'mountain': 'Acherkogel', 'coordinates': [10.95667, 47.18917]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the reference record representation.
{'mountain': 'Acherkogel', 'coordinates': [10.95667, 47.18917]}
tests/engine_test.py
Outdated
| def test_engine_sync_psycopg(): | ||
| """ | ||
| crate+psycopg:// -- Verify connectivity and data transport using the psycopg driver (version 3). | ||
| """ | ||
| engine = sa.create_engine("crate+psycopg://crate@localhost:5432/", isolation_level="AUTOCOMMIT", echo=True) | ||
| assert isinstance(engine, sa.engine.Engine) | ||
| with engine.connect() as connection: | ||
| result = connection.execute(QUERY) | ||
| assert result.mappings().fetchone() == {'mountain': 'Acherkogel', 'coordinates': '(10.95667,47.18917)'} | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_engine_async_psycopg(): | ||
| """ | ||
| crate+psycopg:// -- Verify connectivity and data transport using the psycopg driver (version 3). | ||
| This time, in asynchronous mode. | ||
| """ | ||
| engine = create_async_engine("crate+psycopg://crate@localhost:5432/", isolation_level="AUTOCOMMIT", echo=True) | ||
| assert isinstance(engine, AsyncEngine) | ||
| async with engine.begin() as conn: | ||
| result = await conn.execute(QUERY) | ||
| assert result.mappings().fetchone() == {'mountain': 'Acherkogel', 'coordinates': '(10.95667,47.18917)'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using psycopg, there are deviations, which clearly need to be addressed. By chance, you might have seen this already, @SStorm? Or did you only use asyncpg?
{'mountain': 'Acherkogel', 'coordinates': '(10.95667,47.18917)'}
tests/engine_test.py
Outdated
| @pytest.mark.asyncio | ||
| async def test_engine_async_asyncpg(): | ||
| """ | ||
| crate+asyncpg:// -- Verify connectivity and data transport using the asyncpg driver. | ||
| This exclusively uses asynchronous mode. | ||
| """ | ||
| from asyncpg.pgproto.types import Point | ||
| engine = create_async_engine("crate+asyncpg://crate@localhost:5432/", isolation_level="AUTOCOMMIT", echo=True) | ||
| assert isinstance(engine, AsyncEngine) | ||
| async with engine.begin() as conn: | ||
| result = await conn.execute(QUERY) | ||
| assert result.mappings().fetchone() == {'mountain': 'Acherkogel', 'coordinates': Point(10.95667, 47.18917)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using asyncpg, the response record is not wrong, but its shape is different, as it provides a dedicated Point type for representing coordinates.
{'mountain': 'Acherkogel', 'coordinates': Point(10.95667, 47.18917)}
psycopg and asyncpg driversasyncpg and psycopg drivers
asyncpg and psycopg driversasyncpg and psycopg3 drivers
2f08b2a to
43c45fb
Compare
asyncpg and psycopg3 driversasyncpg and psycopg3 drivers
6ac0a22 to
d2c7613
Compare
5c906ad to
474f658
Compare
474f658 to
dcb6708
Compare
| When using the PostgreSQL protocol with drivers `psycopg` or `asyncpg`, | ||
| the paramstyle is not `qmark`, but `pyformat`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not quite following. I assume we would want to use server side binding (i.e. qmark)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably the reason for needing the workaround at all: Because PostgreSQL drivers psycopg and asyncpg, or the SA dialect, use pyformat, but CrateDB uses qmark, we may need to adjust, iirc.
At least, the patch in its current shape needs it. Maybe there are alternatives to implement it, possibly even easier ones. We will be happy to learn about them.
dcb6708 to
e8bfd77
Compare
e8bfd77 to
f61ebaa
Compare
f61ebaa to
bebbf07
Compare
bebbf07 to
dd73fd7
Compare
08adb59 to
5953f14
Compare
This introduces the `crate+psycopg://`, `crate+asyncpg://`, and `crate+urllib3://` dialect identifiers. The asynchronous variant of `psycopg` is also supported.
5953f14 to
48e6cac
Compare
About
This patch adds support for asyncpg and psycopg3 drivers, by introducing the
crate+asyncpg://andcrate+psycopg://dialect identifiers.It also adds the
crate+urllib3://dialect identifier, for explicitly addressing the urllib3-based transport, in case there might be other HTTP-based transport adapters in the future, using libraries like aiohttp, httpx, or niquests. 1Notes
The asynchronous variant of
psycopgis also supported and will be automatically selected when usingcreate_async_engine()instead ofcreate_engine(), so it doesn't have a dedicated dialect identifier.All of this will only work with SQLAlchemy 2.x.
Installation
pip install 'sqlalchemy-cratedb[all] @ git+https://github.com/crate/sqlalchemy-cratedb@amo/postgresql-async'References
Backlog
psycopg, see below.asyncpgandpsycopg3cratedb-examples#201pandas.read_sql()with both urllib3 vs. psycopg3 cratedb-examples#651Footnotes
Picked up from another discussion at https://github.com/panodata/grafana-client/issues/134. ↩