Skip to content

GH-20339: [C++] Add residual filter support to swiss join - #39487

Merged
westonpace merged 34 commits into
apache:mainfrom
zanmato1984:swiss-join-filter
Mar 12, 2024
Merged

GH-20339: [C++] Add residual filter support to swiss join#39487
westonpace merged 34 commits into
apache:mainfrom
zanmato1984:swiss-join-filter

Conversation

@zanmato1984

@zanmato1984zanmato1984 commented Jan 6, 2024

Copy link
Copy Markdown
Contributor

Rationale for this change

Add residual filter support to swiss join.

What changes are included in this PR?

  1. Added class JoinResidualFilter as a centralized structure to evaluate residual filter in swiss join. It has various flavors of filtering for various join types. Zero-overhead is guaranteed for trivial filters (literal true and sometimes literal false/null). More detailed explanation in code comments.
  2. Tuned the structure of swiss join main body (JoinProbeProcessor::OnNextBatch) to better cope with JoinResidualFilter calls.

Are these changes tested?

Legacy UTs (HashJoin.Random, HashJoin.ResidualFilter and HashJoin.TrivialResidualFilter) cover part of this change. New fine-grained residual filter cases added as well.

Are there any user-facing changes?

No.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #20339has been automatically assigned in GitHub to PR creator.

@zanmato1984zanmato1984 changed the title GH-20339: [C++] Add residual predicate support to swiss joinGH-20339: [C++] Add residual filter support to swiss joinJan 6, 2024
return static_cast<uint32_t>(first_greater - entries) - 1;
}

void SwissTableForJoin::payload_ids_to_key_ids(int num_rows, const uint32_t* payload_ids,

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Deleted because used nowhere in existing code base.

@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 8, 2024
@vibhatha

Copy link
Copy Markdown
Contributor

@zanmato1984 would it be possible to add some test cases?

@zanmato1984

Copy link
Copy Markdown
ContributorAuthor

@zanmato1984 would it be possible to add some test cases?

Yes, I am working on that.

@vibhatha

Copy link
Copy Markdown
Contributor

@zanmato1984 I will completely review once the test cases are there.

@zanmato1984

Copy link
Copy Markdown
ContributorAuthor

I tried to rebase and get things messy. Let me fix my branch first. Sorry for the trouble, guys :(

@zanmato1984

Copy link
Copy Markdown
ContributorAuthor

Hi @westonpace , I've addressed the comments you previously left except one thing I'm uncertain about in my last post. Would you please take a look again? Thanks!

@westonpace
westonpace merged commit 0ce7267 into apache:mainMar 12, 2024
@westonpacewestonpace removed the awaiting merge Awaiting merge label Mar 12, 2024
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 0ce7267.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 11 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][Compute] Add residual predicate support to new (Swiss) hash join

3 participants

@zanmato1984@vibhatha@westonpace