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 test that runs DuckDB its test_all_types function #577

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

JelteF
Copy link
Collaborator

@JelteF JelteF commented Feb 5, 2025

This is an easy way to test conversion from Postgres to DuckDB for types
that don't have an equivalent in Postgres.

It also gives us an easy todo list of types that we don't support yet.

smallint | -32768
int | -2147483648
bigint | -9223372036854775808
hugeint | 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bug! We should be returning:

Suggested change
hugeint | 0
hugeint | -170141183460469231731687303715884105728

$$)
-[ RECORD 1 ]---+-------------------------------------------------------------------------------------------------------------------------------------
bool | f
tinyint | \200
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is displayed pretty weirdly. Maybe we should not use the char type for this, but instead upscale to a regular int. For now I added this as a limitation to the #575 PR.

Comment on lines +42 to +47
date | 07-14-5881580
timestamp | Sun Jan 10 08:01:49.551616 294247
timestamp_s | Sun Jan 10 08:01:49.551616 294247
timestamp_ms | Sun Jan 10 08:01:49.551616 294247
timestamp_ns | Wed Sep 22 00:00:00 1677
timestamp_tz | Sun Jan 10 00:01:49.551616 294247 PST
Copy link
Collaborator Author

@JelteF JelteF Feb 5, 2025

Choose a reason for hiding this comment

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

All of these underflow currently. I think we should either truncate or throw an error in this case.

blob | \000\000\000a
int_array | {42,999,NULL,NULL,-42}
double_array | {42,NaN,Infinity,-Infinity,NULL,-42}
date_array | {01-01-1970,07-11-5881580,07-13-5881580,NULL,05-12-2022}
Copy link
Collaborator Author

@JelteF JelteF Feb 5, 2025

Choose a reason for hiding this comment

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

the two dates in the middle are displayed by duckdb as infinity and -infinity, same for the timestamp_array below. Would be worth looking into what is up with those.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Turns out infinity and -infinity are valid values for dates/timestamps in both Postgres and DuckDB. But based on this our conversion of these is lacking.

@JelteF JelteF force-pushed the test-all-types-duckdb branch from 0ccb600 to 18b655d Compare February 5, 2025 12:02
@JelteF JelteF added this to the 0.4.0 milestone Feb 11, 2025
@JelteF JelteF enabled auto-merge (squash) February 14, 2025 10:19
@JelteF JelteF force-pushed the test-all-types-duckdb branch from b259a10 to f7371be Compare February 14, 2025 10:20
This is an easy way to test conversion from Postgres to DuckDB for types
that don't have an equivalent in Postgres.

It also gives us an easy todo list of types that we don't support yet.
@JelteF JelteF force-pushed the test-all-types-duckdb branch from f7371be to ba4fc36 Compare February 14, 2025 10:20
@mkaruza mkaruza self-requested a review February 14, 2025 10:22
@JelteF JelteF merged commit 02ebeae into main Feb 14, 2025
5 checks passed
@JelteF JelteF deleted the test-all-types-duckdb branch February 14, 2025 10:23
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