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

change lineitem.quantity from integer to double #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ilfrin
Copy link
Member

@ilfrin ilfrin commented Feb 23, 2016

This change is required to conform to the TPC-H spec. The 2.17.1 version
states that the lineitem.l_quantity field should be of decimal type (page
17), as opposed to the currently implemented integer.

I came across this when testing Presto with decimal on TPC-H queries and checking the results.

Not sure if I changed all the necessary bits.

cc @dain

This change is required to conform to the TPC-H spec. The 2.17.1 version
states that the lineitem.l_quantity field should be of decimal type (page
17), as opposed to the currently implemented integer.
@dain
Copy link
Member

dain commented Feb 23, 2016

IIRC, quantity is always a whole number so BIGINT is more appropriate until DECIMAL lands in Presto.

@ilfrin
Copy link
Member Author

ilfrin commented Feb 23, 2016

We can look at this as you say. Then again my original intent was for the schema to conform to the spec. I understand that the generator produces only integers, but quantity in theory may be a fraction as well, which I believe was the intent of the spec. I don't feel strongly either way, so we can land this after decimal lands in Presto, of you prefer that.

Then again, this being a separate airlift project was about abstracting from Presto, so why should we connect that this way :)

This isn't a show stopper, so we can wait.

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

Successfully merging this pull request may close these issues.

2 participants