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

HIVE-26147: OrcRawRecordMerger throws NPE when hive.acid.key.index is missing for an acid file #3219

Merged

Conversation

asolimando
Copy link
Member

@asolimando asolimando commented Apr 17, 2022

See JIRA ticket for details.

The PR is split into distinct commits to ease the review process.

  1. Actual fix
  2. refactoring of TestOrcRawRecordMerger to remove some warnings
  3. refactoring of TestOrcRawRecordMerger to improve readability and allow to more easily add the new test
  4. added unit test to cover the present change
  5. discovered and filed HIVE-26150 while writing the unit test, added comments to clarify why the mock is set up in this way
  6. review comment fix

What changes were proposed in this pull request?

If the metadata is missing, set min/max keys to null (=> full file scan).

Why are the changes needed?

To fix NPE.

Does this PR introduce any user-facing change?

No

How was this patch tested?

mvn test -Dtest=TestOrcRawRecordMerger -pl ql

Copy link

@amansinha100 amansinha100 left a comment

Choose a reason for hiding this comment

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

+1 with a minor comment. Also, pls add the commit message to the PR.

@asolimando asolimando changed the title [WIP] HIVE-26147 orc raw record merger fix HIVE-26147: OrcRawRecordMerger throws NPE when hive.acid.key.index is missing for an acid file Apr 19, 2022
@klcopp klcopp self-requested a review April 19, 2022 15:01
@klcopp klcopp merged commit 414a2f4 into apache:master Apr 19, 2022
@asolimando asolimando deleted the master-HIVE-26147-OrcRawRecordMerger_fix branch April 25, 2022 08:17
DongWei-4 pushed a commit to DongWei-4/hive that referenced this pull request Oct 28, 2022
… missing for an acid file (Alessandro Solimando, reviewed by Aman Sinha and Karen Coppage)

Closes apache#3219.
dengzhhu653 pushed a commit to dengzhhu653/hive that referenced this pull request Dec 15, 2022
… missing for an acid file (Alessandro Solimando, reviewed by Aman Sinha and Karen Coppage)

Closes apache#3219.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants