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.

Sign up for GitHub

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

Jump to bottom

Fix TestHivePlans after logical merge conflict #11433

Merged
merged 1 commit into from
Mar 13, 2022
Merged

Fix TestHivePlans after logical merge conflict #11433

merged 1 commit into from
Mar 13, 2022

Conversation

findepi
Copy link
Member

@findepi findepi commented Mar 11, 2022 •
edited
Loading

70c988b (#11083) and
812f79b (#11086) combined broke the test.

@findepi findepi added the test label Mar 11, 2022
@findepi findepi requested review from losipiuk and sopel39 March 11, 2022 13:15
@cla-bot cla-bot bot added the cla-signed label Mar 11, 2022
@findepi findepi force-pushed the findepi/fix-testhiveplans-087b09 branch from bcad0c5 to cc352a4 Compare March 11, 2022 13:32
@losipiuk
Copy link
Member

70c988b broke the test despite #11083 being green before merging (https://github.com/trinodb/trino/runs/5485787185).

Do you understand why it was green? Was it logical merge conflict? Or we will start getting flakes now?

@findepi findepi changed the title Fix TestHivePlans Fix TestHivePlans after logical merge conflict Mar 11, 2022
@losipiuk
Copy link
Member

70c988b broke the test despite #11083 being green before merging (https://github.com/trinodb/trino/runs/5485787185).

Do you understand why it was green? Was it logical merge conflict? Or we will start getting flakes now?

Ok - I see you changed PR title :)

@findepi
Copy link
Member Author

findepi commented Mar 11, 2022

Yep, i bisected and added explanation now.

sopel39
sopel39 reviewed Mar 11, 2022
View reviewed changes
plugin/trino-hive/src/test/java/io/trino/plugin/hive/optimizer/TestHivePlans.java Outdated
@@ -245,7 +245,7 @@ public void testSubsumePartitionPartWhenOtherFilterNotConvertibleToTupleDomain()
exchange(LOCAL,
exchange(REMOTE, REPARTITION,
project(
filter("R_INT_COL IN (2, 3, 4)",
filter("R_INT_COL IN (2, 3, 4) AND R_INT_COL BETWEEN 2 AND 4",
Copy link
Member

Choose a reason for hiding this comment

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

regression? same issue as in io.trino.plugin.hive.optimizer.TestHivePlans#testSubsumePartitionPartOfAFilter?

Copy link
Member Author

Choose a reason for hiding this comment

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

same response as #10985 (comment) ?
will add a comment too

@findepi findepi force-pushed the findepi/fix-testhiveplans-087b09 branch from cc352a4 to 1616098 Compare March 11, 2022 13:36
@hashhar hashhar merged commit 6c53b97 into trinodb:master Mar 13, 2022
@github-actions github-actions bot added this to the 374 milestone Mar 13, 2022
@findepi findepi deleted the findepi/fix-testhiveplans-087b09 branch March 14, 2022 08:35
@mosabua mosabua mentioned this pull request Mar 14, 2022
Merged
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@losipiuk losipiuk losipiuk approved these changes

@sopel39 sopel39 sopel39 approved these changes

@hashhar hashhar hashhar approved these changes

Assignees
No one assigned
Labels
cla-signed no-release-notes This pull request does not require release notes entry test
Milestone
374
Development

Successfully merging this pull request may close these issues.
4 participants
@findepi @losipiuk @sopel39 @hashhar