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

sql: accept floats when expecting ints during logic tests #3543

Closed
wants to merge 1 commit into from
Closed

sql: accept floats when expecting ints during logic tests #3543

wants to merge 1 commit into from

Conversation

maddyblue
Copy link
Contributor

fixes #3271

Review on Reviewable

@tamird
Copy link
Contributor

tamird commented Dec 29, 2015

This isn't quite enough; i was playing with adding the logic tests enabled by GROUP BY support (#3494) and found for example: SELECT DISTINCT 20 / - 97 FROM tab0 which expects "0" but gets "0.20...". We'll need to convert these floats to integers in this special case.

@tamird
Copy link
Contributor

tamird commented Dec 29, 2015

Actually, even that doesn't work, because of cases like SELECT DISTINCT col1 / 2 FROM tab1 where col1 is an integer; this query will not behave correctly with respect to DISTINCT.

@bdarnell
Copy link
Contributor

What special case, exactly? Are you suggesting that DISTINCT int / int should convert the result to an int, but int / int in other cases should return a float? That seems like a bad idea to me.

The fact that SELECT DISTINCT 20 / - 97 FROM tab0 returns different results for us than it does in other databases is a simple consequence of the fact that we have chosen to have integer division return floats rather than ints; we expect and accept that this decision will not be 100% compatible.

@tamird
Copy link
Contributor

tamird commented Dec 29, 2015

Yes, feel free to ignore the first of my two comments. We will need to either accept this incompatibility and simply skip or massage offending test cases, or else revert to integer division.

@petermattis
Copy link
Collaborator

LGTM


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@maddyblue
Copy link
Contributor Author

We decided to use another solution. Closing.

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.

sql: integer division returns float, sqllogic test expects int
4 participants