Skip to content

Fix LOOKUP join#15223

Merged
yashmayya merged 1 commit intoapache:masterfrom
Jackie-Jiang:fix_lookup_join
Mar 18, 2025
Merged

Fix LOOKUP join#15223
yashmayya merged 1 commit intoapache:masterfrom
Jackie-Jiang:fix_lookup_join

Conversation

@Jackie-Jiang
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang commented Mar 7, 2025

#14893 accidentally removed the workerIdToSegmentsMap and broke LOOKUP join. This PR adds that back. Tested with LookupJoinEngineQuickStart

TODO: Add a query test for LOOKUP join

@Jackie-Jiang Jackie-Jiang added bugfix multi-stage Related to the multi-stage query engine labels Mar 7, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.57%. Comparing base (59551e4) to head (16008f1).
Report is 1826 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15223      +/-   ##
============================================
+ Coverage     61.75%   63.57%   +1.81%     
- Complexity      207     1461    +1254     
============================================
  Files          2436     2761     +325     
  Lines        133233   155613   +22380     
  Branches      20636    23929    +3293     
============================================
+ Hits          82274    98926   +16652     
- Misses        44911    49241    +4330     
- Partials       6048     7446    +1398     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.54% <100.00%> (+1.83%) ⬆️
java-21 63.45% <100.00%> (+1.83%) ⬆️
skip-bytebuffers-false 63.56% <100.00%> (+1.81%) ⬆️
skip-bytebuffers-true 63.43% <100.00%> (+35.70%) ⬆️
temurin 63.57% <100.00%> (+1.81%) ⬆️
unittests 63.56% <100.00%> (+1.82%) ⬆️
unittests1 56.19% <100.00%> (+9.30%) ⬆️
unittests2 34.00% <0.00%> (+6.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@krishan1390
Copy link
Contributor

@Jackie-Jiang Added a test case for this in #15244

Should we merge that PR to this branch ? Or should I wait for this PR to be merged first and then merge the test case to master ?

@yashmayya yashmayya merged commit ad5bb40 into apache:master Mar 18, 2025
22 checks passed
Copy link
Contributor

@yashmayya yashmayya left a comment

Choose a reason for hiding this comment

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

@krishan1390 you can rebase your PR on the latest master and I can then merge yours as well, thanks.

@Jackie-Jiang Jackie-Jiang deleted the fix_lookup_join branch March 18, 2025 19:25
tsekityam pushed a commit to tsekityam/pinot that referenced this pull request Mar 19, 2025
yashmayya pushed a commit to yashmayya/pinot that referenced this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix multi-stage Related to the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants