-
Notifications
You must be signed in to change notification settings - Fork 129
Add boundary check for converting duckdb date/timestamps to PG dates/timestamps #653
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
Conversation
@JelteF
If we dont to throw an error we can use the boundary values for out of range values but we should throw a Also if you have some tips on debugging across C/C++ boundary using gdb (VS code extension seems to not want to work), that would be really appreciated , was trying to walk throught the query lifecycle with pg_duckdb ? |
Thanks for the contribution! Some thoughts:
|
Also, do you know if this also fixes this issue? #629 If so we should include that in the tests. |
With my changes it throws out of bounds Error
|
Hmm, I guess that's better than returning something wrong. But it would be good if we actually converted infinity/-infinity correctly for dates and timestamps, because both databases support it. |
Thanks for the work. This looks almost good, left some mostly cosmetic feedback. CI is also failing due to a compiler error/warning. |
Had to use PG_INT64* Macros for timestamp as specific macros used before were introduces in specific majors only. This will also avoid ugly pg version checks... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the datum conversion functions for the correct types (even though they are functionally equivalent). Other than that this is now good to merge.
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
Postgres and DuckDB support different date and timestamp ranges. This starts to error out when either one receives a value outside of this range. This also starts converting special values for the
infinity
and-infininity
correctly, because these are also not the same between the two databases.Fixes #595
Fixes #612