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

Use table-level SerDe parameter for Avro partitions #14463

Merged
merged 1 commit into from
Oct 24, 2022

Conversation

weijiii
Copy link
Member

@weijiii weijiii commented Oct 4, 2022

Description

Currently, MetastoreUtil::getHiveSchema(Partition, Table) does not include the table's storage descriptor when computing for partitioned tables. We have encountered some cases where Avro table only has schema properties (e.g. avro.schema.literal) stored in the table-level storage descriptor.

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot
Copy link

cla-bot bot commented Oct 4, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: weijiii.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@weijiii weijiii force-pushed the AvroUseTableLevelSd branch from 4d2d094 to 6db08c7 Compare October 4, 2022 19:46
@cla-bot cla-bot bot added the cla-signed label Oct 4, 2022
@weijiii
Copy link
Member Author

weijiii commented Oct 4, 2022

@findepi Please take a look. Thanks!

@weijiii
Copy link
Member Author

weijiii commented Oct 5, 2022

@anusudarsan @arhimondr All checks are passed. I think this is ready for review. Thanks!

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

This change looks fine. Is it feasible to write a test for it? If not, I'm fine to merge without one.

@weijiii
Copy link
Member Author

weijiii commented Oct 21, 2022

This change looks fine. Is it feasible to write a test for it? If not, I'm fine to merge without one.

@electrum Thanks for the review!
After checking we see that this case is still present internally at Linkedin. I am not able to use Hive to reproduce such tables, but from the stance that these functions are to mimic the behavior in Hive, it will be complete if we can merge this.

@phd3 phd3 merged commit 5d50dc0 into trinodb:master Oct 24, 2022
@github-actions github-actions bot added this to the 401 milestone Oct 24, 2022
@weijiii weijiii deleted the AvroUseTableLevelSd branch November 27, 2022 04:20
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.

3 participants