Conversation
lnkuiper
left a comment
There was a problem hiding this comment.
Hi, thanks for the PR! This makes a lot of sense. I just have one request regarding the name of the newly added variable: Can we rename it so the code becomes easier to understand?
| //! Whether or not we error on multiple rows found per match in a SINGLE join | ||
| bool single_join_error_on_multiple_rows = true; | ||
| //! Whether or not to perform deduplication based on join_keys when building ht | ||
| bool deduplicate_keys = false; |
There was a problem hiding this comment.
Can we rename this to insert_duplicate_keys, and default it to true?
There was a problem hiding this comment.
Thanks for your comment, I have renamed the newly added variable.
|
|
||
| if (conditions.size() == 1 && | ||
| (join_type == JoinType::SEMI || join_type == JoinType::ANTI || join_type == JoinType::MARK)) { | ||
| deduplicate_keys = true; |
There was a problem hiding this comment.
insert_duplicate_keys = false;
| for (idx_t i = 0; i < key_match_count; i++) { | ||
| const auto need_compare_idx = state.key_match_sel.get_index(i); | ||
| const auto entry_index = state.keys_to_compare_sel.get_index(need_compare_idx); | ||
| if (!ht.deduplicate_keys) { |
There was a problem hiding this comment.
if (ht.insert_duplicate_keys) {
1ec9b52 to
6abf1f1
Compare
lnkuiper
left a comment
There was a problem hiding this comment.
Thanks for the changes! Looks good to me now, ready when CI passes (not sure why all the runs got cancelled)
|
Thanks for your review! What do I need to do to make CI pass? |
|
Hi @tianjq16, yesterday a bunch of CI runs got cancelled (also on my Fork), not sure what happened. I think we should just run CI again |
|
Thanks! |
Perform deduplication based on join_keys when building ht (duckdb/duckdb#19097)
Perform deduplication based on join_keys when building ht (duckdb/duckdb#19097)
Perform deduplication based on join_keys when building ht (duckdb/duckdb#19097)
Perform deduplication based on join_keys when building ht (duckdb/duckdb#19097)
When building a hash table in the
PhysicalHashJoinoperator, in some cases, if thejoin_keysis duplicated, it is actually not necessary to insert it. For example, thejoin_typeis SEMI and the join condition is a equality condition.