Skip to content

Conversation

Tishj
Copy link
Collaborator

@Tishj Tishj commented Sep 10, 2025

Fixes https://github.com/duckdblabs/duckdb-internal/issues/1217, duckdb/duckdb#10495

This PR should make a better attempt at implementing https://peps.python.org/pep-0249/#type-objects-and-constructors

We add the required type object sentinels:

  • STRING
  • NUMBER
  • DATETIME
  • BINARY
  • ROWID (None, as DuckDB doesn't have a way to detect this on a LogicalType)

The objects returned for the type_code of the description is a DuckDBPyType.
The sentinels are of type DBAPITypeObject and are overloaded to be compared with the DuckDBPyType objects.

The constructors listed by the PEP are not added, they could be added in the future.

Copy link
Collaborator

@evertlammerts evertlammerts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the spec it's not clear what type_code is expected to be. Psycopg returns the type's OID and mysql returns a type id, both of which are numbers, but I don't see a good reason to not return a DuckDBPyType...

@evertlammerts evertlammerts changed the base branch from main to v1.4-andium September 10, 2025 12:58
@Tishj Tishj force-pushed the dbapi2_description_fixes branch from bdda2b6 to 787d12d Compare September 10, 2025 13:17
@Tishj Tishj requested a review from evertlammerts September 10, 2025 14:02
@Tishj Tishj marked this pull request as ready for review September 10, 2025 14:07
Copy link
Collaborator

@evertlammerts evertlammerts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@evertlammerts evertlammerts added this pull request to the merge queue Sep 11, 2025
Merged via the queue into duckdb:v1.4-andium with commit d202b0c Sep 11, 2025
65 of 66 checks passed
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.

2 participants