Skip to content

PHOENIX-6966 UngroupedAggregateRegionScanner.insertEmptyKeyValue() wr…#1617

Closed
stoty wants to merge 1 commit intoapache:masterfrom
stoty:PHOENIX-6966
Closed

PHOENIX-6966 UngroupedAggregateRegionScanner.insertEmptyKeyValue() wr…#1617
stoty wants to merge 1 commit intoapache:masterfrom
stoty:PHOENIX-6966

Conversation

@stoty
Copy link
Copy Markdown
Contributor

@stoty stoty commented Jun 1, 2023

…ites wrong qualifier for encoded CQ tables

@stoty stoty requested a review from ss77892 June 1, 2023 11:18
@gjacoby126
Copy link
Copy Markdown
Contributor

@stoty We're sure that GlobalMutableTxIndexIT.testLastDDLTimestampOnAsyncIndexes with columnEncoding=false failing with an NPE is unrelated?

@stoty
Copy link
Copy Markdown
Contributor Author

stoty commented Jun 2, 2023

Yes I am @gjacoby126 .
I have successfully re-run GlobalMutableTxIndexIT test locally a few times witout problem.

Also, I have checked https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1617/1/artifact/yetus-general-check/output/archiver/phoenix-core/target/failsafe-reports/org.apache.phoenix.end2end.index.GlobalMutableTxIndexIT.txt.gz , and it seems like the minicluster just died /was shutting down when the error happened.

Also, GlobalMutableTxIndexIT doesn't specifically use the changed code:
The added emptyCQ initialization code gets called in most tests for the create table statements,
but as we're always creating new tables in this test, insertEmptyKeyValue() never gets called.

@stoty
Copy link
Copy Markdown
Contributor Author

stoty commented Jun 2, 2023

rebased so that CI can run on this again.
The patch is otherwise unchanged.

Copy link
Copy Markdown
Contributor

@richardantal richardantal left a comment

Choose a reason for hiding this comment

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

LGTM
Locally the test run fine for me.

@stoty stoty closed this Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants