fix extract_signature to work with sql.Composable objects
#148
Conversation
beniwohli
force-pushed
the
pyscopg2-composable-fixes
branch
3 times, most recently
from
60972bf
to
2260ac6
Compare
beniwohli
force-pushed
the
pyscopg2-composable-fixes
branch
from
2260ac6
to
03830a1
Compare
| """ | ||
| Method to turn the "sql" argument into a string | ||
| """ | ||
| return sql |
There was a problem hiding this comment.
The method description might be a bit confusing, as it actually does nothing in this class.
Would it do any harm if you'd actually move the as_string check here?
There was a problem hiding this comment.
the as_string check is specific to psycopg2 (the Python PostgreSQL driver), so I think putting it in the psycopg2-specific subclass makes sense. However I agree that the docstring needs work.
| @@ -121,6 +121,11 @@ def scan(tokens): | |||
|
|
|||
|
|
|||
| def extract_signature(sql): | |||
There was a problem hiding this comment.
Not part of this PR, but out of interest, why is this not a method of CursorProxy ? Afaics this is only used within sql cursor proxy implementations.
There was a problem hiding this comment.
I thought about this as well, but into the other direction: remove CursorProxy.extract_signature and call dbapi2.extract_signature() directly where we need it.
I guess the idea was that if we instrument a SQL backend that needs a different parser, we could plug it in there. @roncohen might remember.
In any case, I'll leave this for another PR :)
There was a problem hiding this comment.
Thank you for giving some context. Might be worth giving it a thought at some point, but as mentioned unrelated to this PR.
see http://initd.org/psycopg/docs/sql.html