Skip to content

Fixed TypeInfo for ClientCursor.#484

Merged
dvarrazzo merged 1 commit intopsycopg:masterfrom
apollo13:typeinfo-for-clientcusor
Jan 16, 2023
Merged

Fixed TypeInfo for ClientCursor.#484
dvarrazzo merged 1 commit intopsycopg:masterfrom
apollo13:typeinfo-for-clientcusor

Conversation

@apollo13
Copy link
Contributor

@apollo13 apollo13 commented Jan 16, 2023

TypeInfo had a hardcoded binary=True which doesn't work to well for ClientCursor.

Without this change we'd get:

Traceback (most recent call last):
  File "/home/florian/sources/django.git/test.py", line 5, in <module>
    print(TypeInfo.fetch(conn, "geometry"))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/florian/.local/share/virtualenvs/django/lib/python3.11/site-packages/psycopg/_typeinfo.py", line 93, in fetch
    cur.execute(cls._get_info_query(conn), {"name": name})
  File "/home/florian/.local/share/virtualenvs/django/lib/python3.11/site-packages/psycopg/cursor.py", line 728, in execute
    raise ex.with_traceback(None)
psycopg.NotSupportedError: client-side cursors don't support binary results

@apollo13 apollo13 force-pushed the typeinfo-for-clientcusor branch 3 times, most recently from 037dd74 to 5cc1a0b Compare January 16, 2023 20:29
@felixxm felixxm mentioned this pull request Jan 16, 2023
Merged
@apollo13
Fixed TypeInfo for ClientCursor.
bfde9f0 
TypeInfo had a hardcoded binary=True which doesn't work to well for
ClientCursor.
@apollo13 apollo13 force-pushed the typeinfo-for-clientcusor branch from 5cc1a0b to bfde9f0 Compare January 16, 2023 20:35
@apollo13
Copy link
Contributor Author

apollo13 commented Jan 16, 2023

Hi @dvarrazzo, I hope this PR is correct -- I couldn't find any reference to why binary=True was used and whether it was needed at some point.

Assuming the tests pass and this PR is merge-able, when do you think you could cut a release with it? I am asking because I'd like to figure out if we should patch around this in Django or can just raise the minimum required version of psycopg in time for Django 4.2.

@dvarrazzo
Copy link
Member

dvarrazzo commented Jan 16, 2023

Hey hello! Thank you very much for the MR!

The binary was there because all the types involved support binary loaders so it was ok to do that, and going in a general direction of trying to use more binary paths. This predates the introduction of the client cursors, and the combination was missing from the test suite.

Thank you very much for adding tests and release notes too!

I can merge and make a new release pretty soon, maybe even tonight.

@dvarrazzo dvarrazzo merged commit f058590 into psycopg:master Jan 16, 2023
@dvarrazzo
Copy link
Member

dvarrazzo commented Jan 16, 2023

@pauloxnet @apollo13 Psycopg 3.1.8 released!

@apollo13
Copy link
Contributor Author

apollo13 commented Jan 17, 2023

Wow, much appreciated. Thank you for the rapid turnaround!

@apollo13 apollo13 deleted the typeinfo-for-clientcusor branch January 17, 2023 08:48
@dvarrazzo dvarrazzo mentioned this pull request May 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@apollo13 @dvarrazzo