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

Add support for REGCLASSOID type #108

Merged
merged 6 commits into from
Aug 8, 2024
Merged

Add support for REGCLASSOID type #108

merged 6 commits into from
Aug 8, 2024

Conversation

Y--
Copy link
Collaborator

@Y-- Y-- commented Aug 8, 2024

Small QoL PR that does two minor thing:

  • shows the Oid when the type in unsupported
  • add mapping for PG's REGCLASSOID type

@Y-- Y-- requested review from Tishj, wuputah, JelteF and mkaruza August 8, 2024 20:37
@wuputah
Copy link
Collaborator

wuputah commented Aug 8, 2024

Could you please add a test case for this type? Once that's done, lgtm!

@Y-- Y-- requested a review from wuputah August 8, 2024 21:45
@Y-- Y-- merged commit b2fd25b into main Aug 8, 2024
2 checks passed
@Y-- Y-- deleted the yl/regclassoid branch August 8, 2024 21:46
default:
return duckdb::LogicalType::USER("UnsupportedPostgresType");
case REGCLASSOID:
return duckdb::LogicalTypeId::INTEGER;
Copy link
Collaborator

Choose a reason for hiding this comment

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

That works, do keep in mind that this means we lose the context of what this was on the postgres side this way.
We'd need something similar to what we do for NumericAsDouble to differentiate between an actual INT4 and a REGCLASSOID

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah definitely. I added this because it was useful for me debugging internals but agree that we're losing information here.

@hlinnaka
Copy link
Collaborator

hlinnaka commented Aug 9, 2024

OIDs are unsigned, so this goes wrong:

postgres=# create table x (cls regclass);
CREATE TABLE
postgres=# insert into x values (3_000_000_000);
INSERT 0 1
postgres=# select * from x;
    cls     
------------
 3000000000
(1 row)

postgres=# SET duckdb.execution TO true;
SET
postgres=# select * from x;
     cls     
-------------
 -1294967296
(1 row)

@Y--
Copy link
Collaborator Author

Y-- commented Aug 13, 2024

OIDs are unsigned, so this goes wrong:

postgres=# create table x (cls regclass);
CREATE TABLE
postgres=# insert into x values (3_000_000_000);
INSERT 0 1
postgres=# select * from x;
    cls     
------------
 3000000000
(1 row)

postgres=# SET duckdb.execution TO true;
SET
postgres=# select * from x;
     cls     
-------------
 -1294967296
(1 row)

Thanks for catching this! I'll work on this ASAP

@Y-- Y-- mentioned this pull request Aug 14, 2024
@Y--
Copy link
Collaborator Author

Y-- commented Aug 14, 2024

Will be fixed here: #125

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.

4 participants