Skip to content

[fix](join) incorrect result of right anti join with nullable #28258

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
Merged
yiguolei merged 1 commit into from
Dec 13, 2023
Merged

[fix](join) incorrect result of right anti join with nullable #28258

yiguolei merged 1 commit into from
Dec 13, 2023

Conversation

@mrhhsg
Copy link
Member

@mrhhsg mrhhsg commented Dec 11, 2023

Proposed changes

If the probe column on the left side is not nullable and the build column on the right is nullable, the anti join's result is incorrect.

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@mrhhsg
Copy link
Member Author

mrhhsg commented Dec 11, 2023

run buildall

@wm1581066 wm1581066 added dev/2.0.4 usercase Important user case type label labels Dec 11, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Dec 11, 2023

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

doris-robot commented Dec 11, 2023

TeamCity be ut coverage result:
Function Coverage: 37.82% (7987/21118)
Line Coverage: 29.52% (64815/219590)
Region Coverage: 28.98% (33345/115057)
Branch Coverage: 24.87% (17115/68806)
Coverage Report: http://coverage.selectdb-in.cc/coverage/5bbdd9f59f862afeb34714637295e8362c94d46c_5bbdd9f59f862afeb34714637295e8362c94d46c/report/index.html

@doris-robot
Copy link

doris-robot commented Dec 11, 2023

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 49.66 seconds
stream load tsv: 585 seconds loaded 74807831229 Bytes, about 121 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 29.6 seconds inserted 10000000 Rows, about 337K ops/s
storage size: 17162318074 Bytes

@mrhhsg mrhhsg force-pushed the fix_right_anti_join branch from 5bbdd9f to aa8be36 Compare December 11, 2023 15:45
@mrhhsg
Copy link
Member Author

mrhhsg commented Dec 11, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 11, 2023

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

doris-robot commented Dec 11, 2023

TeamCity be ut coverage result:
Function Coverage: 37.80% (7983/21118)
Line Coverage: 29.50% (64785/219600)
Region Coverage: 28.96% (33325/115062)
Branch Coverage: 24.86% (17107/68810)
Coverage Report: http://coverage.selectdb-in.cc/coverage/aa8be36527471f635d4642a1dd339061e0dc6993_aa8be36527471f635d4642a1dd339061e0dc6993/report/index.html

@mrhhsg
Copy link
Member Author

mrhhsg commented Dec 11, 2023

run buildall

@doris-robot
Copy link

doris-robot commented Dec 11, 2023

TeamCity be ut coverage result:
Function Coverage: 37.82% (7987/21118)
Line Coverage: 29.52% (64820/219600)
Region Coverage: 28.98% (33349/115062)
Branch Coverage: 24.88% (17121/68810)
Coverage Report: http://coverage.selectdb-in.cc/coverage/aa8be36527471f635d4642a1dd339061e0dc6993_aa8be36527471f635d4642a1dd339061e0dc6993/report/index.html

@doris-robot
Copy link

doris-robot commented Dec 11, 2023

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 50.73 seconds
stream load tsv: 587 seconds loaded 74807831229 Bytes, about 121 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 30.0 seconds inserted 10000000 Rows, about 333K ops/s
storage size: 17164368302 Bytes

BiteTheDDDDt
BiteTheDDDDt reviewed Dec 12, 2023
View reviewed changes
be/src/vec/exec/join/vhash_join_node.cpp
}
} else {
if constexpr (!BuildSide) {
if (_join_op == TJoinOp::RIGHT_ANTI_JOIN &&
Copy link
Contributor

@BiteTheDDDDt BiteTheDDDDt Dec 12, 2023

Choose a reason for hiding this comment

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

Does only right anti join need to be converted?

Copy link
Member Author

@mrhhsg mrhhsg Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, right join and full join will be converted in _convert_block_to_null.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 12, 2023

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 12, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Dec 12, 2023

PR approved by anyone and no changes requested.

@mrhhsg mrhhsg force-pushed the fix_right_anti_join branch from aa8be36 to a2b08aa Compare December 12, 2023 10:29
@mrhhsg
Copy link
Member Author

mrhhsg commented Dec 12, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 12, 2023

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

doris-robot commented Dec 12, 2023

TeamCity be ut coverage result:
Function Coverage: 37.81% (7985/21118)
Line Coverage: 29.51% (64799/219600)
Region Coverage: 28.97% (33334/115062)
Branch Coverage: 24.87% (17112/68810)
Coverage Report: http://coverage.selectdb-in.cc/coverage/a2b08aa870b6ac9dbe233dd3d3f9a71d6b89b324_a2b08aa870b6ac9dbe233dd3d3f9a71d6b89b324/report/index.html

HappenLee
HappenLee approved these changes Dec 12, 2023
View reviewed changes
Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

@mrhhsg mrhhsg mentioned this pull request Dec 12, 2023
Merged
@mrhhsg mrhhsg force-pushed the fix_right_anti_join branch from a2b08aa to e64cd46 Compare December 12, 2023 23:56
@mrhhsg
Copy link
Member Author

mrhhsg commented Dec 12, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 13, 2023

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

doris-robot commented Dec 13, 2023

TeamCity be ut coverage result:
Function Coverage: 37.82% (7986/21118)
Line Coverage: 29.51% (64813/219629)
Region Coverage: 28.97% (33337/115070)
Branch Coverage: 24.87% (17111/68810)
Coverage Report: http://coverage.selectdb-in.cc/coverage/e64cd4605706474b7c6a49bb0a6102e49f7c368a_e64cd4605706474b7c6a49bb0a6102e49f7c368a/report/index.html

@yiguolei yiguolei merged commit 09501d0 into apache:branch-2.0 Dec 13, 2023
@mrhhsg mrhhsg deleted the fix_right_anti_join branch January 10, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BiteTheDDDDt BiteTheDDDDt BiteTheDDDDt approved these changes

@HappenLee HappenLee HappenLee approved these changes

@yiguolei yiguolei Awaiting requested review from yiguolei

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/2.0.4-merged kind/test p0_w reviewed usercase Important user case type label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@mrhhsg @doris-robot @BiteTheDDDDt @HappenLee @xiaokang @yiguolei @wm1581066