Fixed #14, implemented prepared statement #219
Conversation
Pull Request Test Coverage Report for Build 818
💛 - Coveralls |
Pull Request Test Coverage Report for Build 833
💛 - Coveralls |
gino/dialects/base.py
Outdated
| params = [] | ||
| for val in ctx.parameters[0]: | ||
| if asyncio.iscoroutine(val): | ||
| val = await val |
There was a problem hiding this comment.
this should be taken care at call sites?
There was a problem hiding this comment.
Urh yeah you're probably right about it. It was a necessary await for _ResultProxy.execute() because it is a workaround for the schema generator. But no one should have used prepared statement yet, so I'll remove this one.
| self.clause, *multiparams, **params).context | ||
| if ctx.executemany: | ||
| raise ValueError('PreparedStatement does not support multiple ' | ||
| 'parameters.') |
There was a problem hiding this comment.
Because asyncpg doesn't have it on prepared statement. User who wants to use executemany() with prepared statement has to fall back to sequential all() calls, or not to use prepared statement to save RTT.
There was a problem hiding this comment.
Thx for the info. I looked up, and it seems to be due to PostgreSQL.
(I saw some "workaround" https://stackoverflow.com/a/14903130/238634 but it'd make it too complex)
There was a problem hiding this comment.
Hmm it's more complicated than I thought.
First of all, current executemany() is not saving RTT, it is only saving some slow Python time: MagicStack/asyncpg#36
And yes, extended query protocol (video) does not support multiple SQL statements, but what we need is actually multiple Bind and Execute commands, ideally issued in a batch. Under the hood prepared statement is actually named statement of extended query, so it should be possible to use prepared statement with executemany(), asyncpg just didn't expose the interface.
So for the first issue, I tried to tweak asyncpg a bit to send Bind and Execute in one packet, it just worked fine. And for the second issue, it won't be hard to add executemany() in prepared statement I suppose (commit).
As for this PR, guess I'll leave as it is now, and note about upstream.
|
Thanks for @wwwjfy your comments! I've submitted MagicStack/asyncpg#289 for comments. |
Anyone feels like reviewing this one? It's a bit hacky, but works at least. It is really a pain that SQLAlchemy does not support prepared statement at all.