Skip to content

Conversation

@jackkleeman
Copy link
Contributor

It should be possible to compare numeric types to uint64 literals. Currently for anything above the int64 max, it will become a float or a decimal128 depending on config. The float conversion is lossy, and the decimal type is currently not comparable with uint64, so both of these are problematic.

Which issue does this PR close?

Closes #6145.

Are these changes tested?

Yes

Are there any user-facing changes?

Potentially this will affect how some queries are interpreted, where int literals greater than int64-max are provided.

It should be possible to compare numeric types to uint64 literals. Currently for anything above the int64 max, it will become a float or a decimal128 depending on config. The float conversion is lossy, and the decimal type is currently not comparable with uint64, so both of these are problematic.
@github-actions github-actions bot added the sql SQL Planner label Apr 27, 2023
@yukkit
Copy link
Contributor

yukkit commented Apr 28, 2023

LGTM

@alamb
Copy link
Contributor

alamb commented Apr 28, 2023

THanks @jackkleeman and @yukkit -- I have kicked off the CI

))
} else if let Ok(n) = n.parse::<i64>() {
Ok(lit(n))
} else if let Ok(n) = n.parse::<u64>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@jackkleeman please run cargo fmt --all and resubmit the PR

@alamb
Copy link
Contributor

alamb commented Apr 30, 2023

I took the liberty of fixing the cargo fmt issue and merging up from main

@alamb alamb merged commit ab8991a into apache:main Apr 30, 2023
@alamb
Copy link
Contributor

alamb commented Apr 30, 2023

Thanks again @jackkleeman

@jackkleeman jackkleeman deleted the add-uint64-literal branch May 2, 2023 08:12
@jackkleeman
Copy link
Contributor Author

thanks folks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support uint64 literals

4 participants