Skip to content
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

postgresql_info: fix SQL error of issue #790 #791

Closed
wants to merge 1 commit into from
Closed

Conversation

betanummeric
Copy link
Member

fixing #790

The dynamic query construction fails because the column type is bytes in python, but the code expects str. The python conversion from bytes to str addess extra characters which break the SQL syntax of the following query.
I guess psycopg 2 vs 3 behave different regarding types, which was unexpected in the code.

This fix using the quote_ident function converts the type in SQL which should result in str in python. By the way it adds support for arbitrary column names (although this is rather unlikely to become relevant in a table of pg_catalog).

postgres=# SELECT pg_typeof(quote_ident(column_name)), pg_typeof(column_name) FROM information_schema.columns WHERE table_schema = 'pg_catalog' AND table_name = 'pg_subscription' limit 1;
 pg_typeof |             pg_typeof             
-----------+-----------------------------------
 text      | information_schema.sql_identifier

@betanummeric
Copy link
Member Author

I didn't test the change yet, I can't promise to do that soon. If someone wants to take this PR over from here - feel free.

@Andersson007
Copy link
Collaborator

I didn't test the change yet, I can't promise to do that soon. If someone wants to take this PR over from here - feel free.

@betanummeric thanks for the PR! i think it's OK to wait generally:) thanks for being explicit

@Andersson007
Copy link
Collaborator

@betanummeric i played with it manually, and it doesn't work
If you look at possible solutions, it'd be great:

@Andersson007
Copy link
Collaborator

@betanummeric it turned out that the issue is hard to catch. I'll close this PR as i did with related mine. Feel free to re-open if you think it's relevant. Thanks for investigating and submitting it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants