Skip to content

Fix column type OIDs in pgduck_server row description metadata#327

Open
Khoulii wants to merge 5 commits into
Snowflake-Labs:mainfrom
Khoulii:main
Open

Fix column type OIDs in pgduck_server row description metadata#327
Khoulii wants to merge 5 commits into
Snowflake-Labs:mainfrom
Khoulii:main

Conversation

@Khoulii
Copy link
Copy Markdown

@Khoulii Khoulii commented Apr 22, 2026

Description

Fixes #67

Currently, duckdb_query_result_send_column_metadata always writes InvalidOid (0) as the column type OID in the row description message, regardless of the actual column type. This breaks clients that rely on PQftype() for type introspection (e.g. ORMs, psycopg2, JDBC drivers).
This PR adds duckdb_type_to_pg_oid() in type_conversion.c which maps duckdb_type values to their closest PostgreSQL OID equivalents, and wires it into the column metadata sender in place of the hardcoded InvalidOid.

Mapping strategy:

  • Exact matches where sizes align: BOOLEAN→BOOLOID, SMALLINT→INT2OID, INTEGER→INT4OID, BIGINT→INT8OID, FLOAT→FLOAT4OID, DOUBLE→FLOAT8OID, DATE→DATEOID, TIME→TIMEOID, TIMESTAMP→TIMESTAMPOID, TIMESTAMPTZ→TIMESTAMPTZOID, INTERVAL→INTERVALOID, UUID→UUIDOID, BLOB→BYTEAOID
  • Unsigned types promoted to the next signed PG size that fits (UTINYINT→INT2OID, USMALLINT→INT4OID, UINTEGER→INT8OID)
  • Types with no native PG equivalent (UBIGINT, HUGEINT, UHUGEINT, DECIMAL) → NUMERICOID
  • Complex/nested types (LIST, ARRAY, STRUCT, MAP, UNION, ENUM) → TEXTOID since they are serialised as text. These types should be considered to be handled more properly in future PRs.

columnLength and columnTypeMod remain -1; NUMERIC typmod encoding (precision/scale) is left as a follow-up.


Checklist

  • I have tested my changes and added tests if necessary
  • I updated documentation if needed
  • I confirm that all my commits are signed off (DCO)

Khaled Alkhouli and others added 2 commits April 23, 2026 00:27
…validOid

Signed-off-by: Khoulii <k.khouli@yahoo.com>
…Snowflake-Labs#326)

When PQconsumeInput() returned false (broken connection), WaitForResult
called ReleasePGDuckConnection() before re-throwing the error.  The
PG_FINALLY block in ExecuteCopyToCommandOnPGDuckConnection (and every
other caller that wraps GetPGDuckConnection/ExecuteQueryOnPGDuckConnection
in a PG_TRY) then called ReleasePGDuckConnection() a second time on the
same hash entry.

After HASH_REMOVE the entry's memory is returned to dynahash's freelist.
If the slot was reused for the retry connection created inside
ExecuteQueryOnPGDuckConnection, the second call freed the *new* connection.
If the slot was not reused the second PQfinish() was called on an already-
freed PGconn, producing the bogus address seen in the crash:

  #0  __GI___libc_free (mem=0xf5c9c67aca47e118) at malloc.c:3375
  Snowflake-Labs#1  pqReleaseConnHosts (conn=0x30e138c0) at fe-connect.c:4723
  Snowflake-Labs#4  ReleasePGDuckConnection at src/pgduck/client.c:186

Fix: remove the ReleasePGDuckConnection() call from WaitForResult.
Connection lifetime is exclusively the caller's responsibility, managed
via the PG_FINALLY block.

Signed-off-by: Khoulii <k.khouli@yahoo.com>
Copy link
Copy Markdown
Collaborator

@sfc-gh-mslot sfc-gh-mslot left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this! Did you run into a specific issue with the invalid OIDs?

Comment thread pgduck_server/src/duckdb/type_conversion.c Outdated
Comment thread pgduck_server/src/duckdb/type_conversion.c Outdated
@Khoulii
Copy link
Copy Markdown
Author

Khoulii commented Apr 28, 2026

Thanks for looking into this! Did you run into a specific issue with the invalid OIDs?

@sfc-gh-mslot
thank you for the follow up,
the current behavior is to always return invalid OIDs, which is not a good thing to have.
I acknowledge that in production environment this won't be an issue as pgduck_server is not handling the requests, as it will be processed by the actual pg server.
but for local/dev and some CI/CD it's good to have a well-behaving duckdb over postgres wire protocol without bearing the burden of a heavy weight pg server.

this issue reports that Grafana was unable to draw graphs ..etc. because of the bad OIDs.

again, we did not observe this on production but it's very convenient for local/dev use cases.

@Khoulii Khoulii requested a review from sfc-gh-mslot April 29, 2026 22:12
Comment thread pgduck_server/src/duckdb/duckdb.c Outdated
Khoulii added 2 commits May 11, 2026 15:46
…id instead of TEXTOID.

Signed-off-by: Khoulii <k.khouli@yahoo.com>
Signed-off-by: Khoulii <k.khouli@yahoo.com>
@sfc-gh-dachristensen
Copy link
Copy Markdown
Collaborator

Triggering PR vaildation now. Thanks for the contribution!

@sfc-gh-mslot
Copy link
Copy Markdown
Collaborator

@sfc-gh-dachristensen this requires extensive test changes

I think I got most of them at
https://github.com/Snowflake-Labs/pg_lake/tree/marcoslot/fix-column-oids-327

@sfc-gh-dachristensen
Copy link
Copy Markdown
Collaborator

@sfc-gh-mslot branch looks good; we want to force push here?

sfc-gh-mslot pushed a commit that referenced this pull request May 22, 2026
Map DuckDB types to their proper PostgreSQL type OIDs in the row
description metadata, instead of returning InvalidOid (which downstream
clients silently coerce to TEXTOID and lose type information).

Originally submitted as #327 by @Khoulii. Transplanted onto a maintainer
branch to run the full CI suite (which only triggers on push, not on
pull_request from forks).

Co-authored-by: Khoulii <k.khouli@yahoo.com>
Signed-off-by: Khaled Alkhouli <khaled.alkhouli@opensooq.com>
Signed-off-by: Khoulii <k.khouli@yahoo.com>
Signed-off-by: Marco Slot <marco.slot@snowflake.com>
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.

pgduck_server does not seem to implement/pass types with "\gdesc"

3 participants