Fix lookup join returning 0 rows when a dimension primary-key component is a literal [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] - #19197
Conversation
…nt is a literal When a lookup join condition includes a literal on a dimension table primary key column (e.g. dim_tbl.currency = 'gbp'), Calcite's analyzeCondition() classifies it as a non-equi condition rather than an equi-join key. The LookupJoinOperator builds the lookup key only from leftKeys (equi-join column-column pairs), so the literal component is missing from the key, making it shorter than the dimension table's primary key. The lookup always returns null, producing 0 result rows. Fix: Build the lookup key in the dimension table's primary key column order. For each primary key column, determine the value source: either the corresponding left column (equi-join via leftKeys/rightKeys) or a literal value extracted from the non-equi condition. Closes apache#19188
Jackie-Jiang
added
bug
There was a problem hiding this comment.
Pull request overview
Fixes lookup-join key construction when a dimension-table primary key component is provided as a literal in the join condition (e.g. dim.currency = 'gbp'), by building the lookup PrimaryKey in the dimension PK column order and populating missing components from literal non-equi predicates.
Changes:
- Build lookup keys in dimension PK order, filling each PK component from either equi-join mappings or extracted
dim_col = literalpredicates. - Add a regression test case covering a literal PK component in the lookup join condition.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LookupJoinOperator.java | Reworks lookup-key construction to include literal dimension PK components extracted from non-equi predicates. |
| pinot-query-runtime/src/test/resources/queries/LookupJoin.json | Adds lookup_join_literal_key regression coverage for literal PK component joins. |
Suppressed comments (1)
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LookupJoinOperator.java:110
_rightKeyIdsis being populated here but is never used later. The extra field and initialization add noise and should be removed.
List<Integer> rightKeys = node.getRightKeys();
_rightKeyIds = new int[rightKeys.size()];
for (int i = 0; i < rightKeys.size(); i++) {
_rightKeyIds[i] = rightKeys.get(i);
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| List<String> primaryKeyColumns = _rightTable.getPrimaryKeyColumns(); | ||
| Preconditions.checkState(primaryKeyColumns != null && !primaryKeyColumns.isEmpty(), | ||
| "Dimension table must have primary key columns for lookup join"); | ||
| _keyLeftIndices = new int[primaryKeyColumns.size()]; |
| private final int[] _leftKeyIds; | ||
| private final int[] _rightKeyIds; |
| @@ -64,6 +65,12 @@ public class LookupJoinOperator extends MultiStageOperator { | |||
| private final LeafOperator _rightInput; | |||
| private final JoinRelType _joinType; | |||
| private final int[] _leftKeyIds; | |||
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
cc @gortiz @yashmayya to help review the change
This should be quite rare, so we should try to make the overhead minimal for regular cases where no extra filter exists
| for (RexExpression nonEquiCondition : nonEquiConditions) { | ||
| if (nonEquiCondition instanceof RexExpression.FunctionCall) { | ||
| RexExpression.FunctionCall functionCall = (RexExpression.FunctionCall) nonEquiCondition; | ||
| if ("EQUALS".equals(functionCall.getFunctionName()) && functionCall.getFunctionOperands().size() == 2) { |
There was a problem hiding this comment.
This is not general enough. Essentially we want to support extra filters on looked-up records, and it is not limited to EQUALS
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19197 +/- ##
=============================================
- Coverage 66.65% 38.90% -27.76%
+ Complexity 1423 1422 -1
=============================================
Files 3443 3443
Lines 218632 218674 +42
Branches 34793 34808 +15
=============================================
- Hits 145726 85069 -60657
- Misses 61192 125843 +64651
+ Partials 11714 7762 -3952
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
When a lookup join condition includes a literal on a dimension table primary key column (e.g.
dim_tbl.currency = 'gbp'), Calcite'sanalyzeCondition()classifies it as a non-equi condition rather than an equi-join key. TheLookupJoinOperatorbuilds the lookup key only fromleftKeys(equi-join column-column pairs), so the literal component is missing from the key. The key becomes shorter than the dimension table's primary key, causing the lookup to always return null — producing 0 result rows.Root Cause
The condition
dim_tbl.currency = 'gbp' AND dim_tbl.rate_start_date = fact_tbl.rate_start_dateis split by Calcite as:dim_tbl.rate_start_date = fact_tbl.rate_start_date→leftKeys/rightKeysdim_tbl.currency = 'gbp'(column vs literal)The operator builds the key from
leftKeysonly, producing a 1-component key. But the dimension table's primary key is[currency, rate_start_date](2 components), so the lookup always fails.Fix
Build the lookup key in the dimension table's primary key column order, filling each position from either:
leftKeys/rightKeysmapping)dim_col = literalTesting
Added
lookup_join_literal_keytest case toLookupJoin.jsonwith:[currency, rate_start_date]dim_tbl.currency = 'gbp' AND dim_tbl.rate_start_date = fact_tbl.rate_start_date['gbp', 125]Closes #19188