Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
(#4462) Postgres compatibility tests using sqllogictest #4834
(#4462) Postgres compatibility tests using sqllogictest #4834
Changes from 16 commits
236bdc8
d00a1de
8150314
d5369d0
d06ef0f
f8c122b
ff1e089
55a49af
1a78bc6
ad4acb5
fa7866a
70aa1c9
9bee2a3
4de00da
de741df
44bb1de
b7b9e70
8453ed1
5c2fc55
4c7734d
81500c8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Here are the reasons for these dependencies:
half
-f16
type for Datafusiontestcontainers
- creates a fresh docker container with Postgres for each sqllogictest file.postgres-types
andtokio-postgres
- these are required for writing a Postgres clientrust_decimal
- converts Postgres "numeric" type to a rust typebigdecimal
- provides a common type to do floating number rounding.rust_decimal
, unfortunately, doesn't handle numbers of arbitrary precision. For example,rust_decimal
could not parse26156334342021890000000000000000000000
that is currently present in one of.slt
tests in Datafusion.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.
All numbers are rounded to
12
decimal digits. Without explicit types Postgres and Datafusion can choose different underlying types. For example, Postgres could choose to usenumeric
when Datafusion usesint
. In order to compare results, all floating number types are converted to the same number of decimal points.12
is chosen to pass the existing set of tests. I think it could produce errors, for example, when roundingf16
to 12 digits. I would probably use3
(or4
) decimal digits if high precision is not required for Postgres compatibility tests.3
or4
is an expected number of digits for 16 bit binary according to IEEE_754, so it should be safer to round to the smallest possible data type.