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

Support decimal casting in Parquet #2823

Merged
merged 4 commits into from Feb 27, 2020

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented Feb 13, 2020

Supports converting between ShortDecimals <-> LongDecimal when reading from Parquet Decimal column into Presto decimal column.

When Presto scale is < Parquet scale then value got rounded.

In every other case this code checks if converted value overflows Presto decimal type (if it does - exception is thrown) or got truncated on significant side (it it does - exception is thrown).

@cla-bot cla-bot bot added the cla-signed label Feb 13, 2020
@wendigo wendigo force-pushed the parquet_decimal_rescaling branch 6 times, most recently from 5da854a to 2d32ebc Compare February 14, 2020 11:48
@wendigo wendigo force-pushed the parquet_decimal_rescaling branch 2 times, most recently from 4df795e to c537673 Compare February 20, 2020 16:09
@wendigo wendigo changed the title Support decimal rescaling in Parquet Support decimal casting in Parquet Feb 20, 2020
Copy link
Member

@aalbu aalbu left a comment

Choose a reason for hiding this comment

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

Looks good to me. I like the tests.

@wendigo wendigo force-pushed the parquet_decimal_rescaling branch 5 times, most recently from fda3390 to 8363fb5 Compare February 21, 2020 11:26
Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

Overall looks good. Editorial comments

@wendigo wendigo force-pushed the parquet_decimal_rescaling branch 4 times, most recently from c53a53a to 5070b1b Compare February 23, 2020 19:25
@wendigo wendigo force-pushed the parquet_decimal_rescaling branch 2 times, most recently from 90501fe to 7ca75ea Compare February 26, 2020 14:08
@wendigo wendigo requested a review from findepi February 26, 2020 14:27
@findepi findepi merged commit e36dbff into trinodb:master Feb 27, 2020
@findepi
Copy link
Member

findepi commented Feb 27, 2020

Merged, thanks!

@findepi findepi added this to the 331 milestone Feb 27, 2020
@findepi findepi mentioned this pull request Feb 27, 2020
6 tasks
@wendigo wendigo deleted the parquet_decimal_rescaling branch March 9, 2020 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants