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

Fix sequence encodings in Checkpoints.getStreamCheckpoints #158

Merged
merged 1 commit into from Feb 8, 2019

Conversation

4 participants
@kokosing
Copy link
Member

kokosing commented Feb 5, 2019

Fix sequence encodings in Checkpoints.getStreamCheckpoints

DWRF sequence encoding was introduced in
87f7e51 to support read DWRF flat map.

However, Checkpoints.getStreamCheckpoints still uses the column level
encoding, even for flat map.

This causes failure when reading from a DWRF flat map where
the DWRF sequence use different encodings for different keys.
(for example, some key uses dictionary encoding while some other
keys use direct encoding)

Extracted from:
prestodb/presto@eb011ec

Fix sequence encodings in Checkpoints.getStreamCheckpoints
DWRF sequence encoding was introduced in
87f7e51 to support read DWRF flat map.

However, Checkpoints.getStreamCheckpoints still uses the column level
encoding, even for flat map.

This causes failure when reading from a DWRF flat map where
the DWRF sequence use different encodings for different keys.
(for example, some key uses dictionary encoding while some other
keys use direct encoding)

Extracted from:
prestodb/presto@eb011ec
@cla-bot

This comment has been minimized.

Copy link

cla-bot bot commented Feb 5, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@prestosql.io. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@findepi findepi requested a review from dain Feb 5, 2019

@dain

This comment has been minimized.

Copy link
Member

dain commented Feb 6, 2019

We can merge this, but since there is no spec or alternate implementation for DWRF, we have no way to verify the correctness of the change.

@dain

dain approved these changes Feb 7, 2019

@kokosing kokosing merged commit 9d02e43 into prestosql:master Feb 8, 2019

1 check failed

verification/cla-signed
Details

@kokosing kokosing deleted the kokosing:origin/master/078_Checkpoints.getStreamCheckpoints branch Feb 8, 2019

@kokosing

This comment has been minimized.

Copy link
Member Author

kokosing commented Feb 8, 2019

@dain Would you like add release notes for this?

@dain

This comment has been minimized.

Copy link
Member

dain commented Feb 8, 2019

I added the release notes

@kokosing

This comment has been minimized.

Copy link
Member Author

kokosing commented Feb 8, 2019

Thanks!

@electrum electrum added this to the 303 milestone Feb 14, 2019

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