fix: incorporate literal values into lookup join key

[waterWang]

Description

Fixes a bug in LookupJoinOperator where a lookup join returns 0 rows when a dimension table primary-key component is supplied as a literal in the join condition.

Root Cause

When a join condition like dim.currency = 'gbp' AND dim.rate_start_date = fact.rate_start_date is used, Calcite's analyzeCondition() classifies the dim.currency = 'gbp' part as a non-equi condition (right-column-to-literal equality, not a left-right column equality). The LookupJoinOperator then builds the lookup key using only the left-key columns (rate_start_date), resulting in an incomplete key that doesn't match the dimension table's composite primary key ([currency, rate_start_date]). The lookup returns null, and the join produces 0 rows.

Fix

In LookupJoinOperator's constructor, analyze the non-equi conditions to find = equality expressions where one operand is an InputRef (pointing to a right-side column) and the other is a Literal. When the right-side column is part of the dimension table's primary key, use the literal value as a key component. This ensures the lookup key is complete and matches the dimension table's primary key.

Key changes

• LookupJoinOperator.java: Add _keyColumnCount, _keyColumnLeftIds, and _keyColumnLiteralValues fields. In the constructor, extract literal-based key components from non-equi conditions. Update getKey() and fillKey() to build the complete primary key including literals. Update buildJoinedDataBlockSemi and buildJoinedDataBlockAnti to use the full key size.
• LookupJoin.json: Add a test case lookup_join_literal_key that reproduces the bug.

Test

SELECT /*+ joinOptions(join_strategy='lookup') */ {dim_tbl}.currency, {dim_tbl}.rate
FROM {fact_tbl} JOIN {dim_tbl}
ON {dim_tbl}.currency = 'gbp' AND {dim_tbl}.rate_start_date = {fact_tbl}.rate_start_date

Expected: [["gbp", 125]] (1 row)
Before fix: [] (0 rows)

Closes #19188

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.89%. Comparing base (3ffb614) to head (1c718aa).

Files with missing lines	Patch %	Lines
...not/query/runtime/operator/LookupJoinOperator.java	0.00%	53 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (3ffb614) and HEAD (1c718aa). Click for more details.

HEAD has 4 uploads less than BASE

Flag	BASE (3ffb614)	HEAD (1c718aa)
java-25	6	5
temurin	6	5
unittests1	1	0
unittests	2	1

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #19200       +/-   ##
=============================================
- Coverage     66.63%   38.89%   -27.74%     
+ Complexity     1423     1422        -1     
=============================================
  Files          3443     3443               
  Lines        218663   218709       +46     
  Branches      34801    34817       +16     
=============================================
- Hits         145705    85075    -60630     
- Misses        61230   125875    +64645     
+ Partials      11728     7759     -3969     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	38.89% <0.00%> (-27.74%)	⬇️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	38.89% <0.00%> (-27.74%)	⬇️
unittests	38.89% <0.00%> (-27.74%)	⬇️
unittests1	?
unittests2	38.89% <0.00%> (-0.01%)	⬇️

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

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[timothy-e]

Thanks for starting on this so fast! I I wasn't sure if you're ready for review or not, but gave it a pass through.

The idea makes sense to me, but have a few small concerns

[timothy-e]

this map is unused

[timothy-e]

This could cause a literal to overwrite a equi-join key, e.g. in a scenario like this:

  ON fact.currency = dim.currency
  AND fact.rate_start_date = dim.rate_start_date
  AND dim.currency = 'gbp'

The join would only validate dim.currency=gbp, not fact.currency=dim.currency. Can you also add a test for this case?

[timothy-e]

This uses the same representation for “unassigned PK component” and “literal NULL”, and makes missing PK slots look intentionally mapped to NULL, which changes the behaviour more than what we probably intend to do.

(e.g. right now, what happens when a lookup join doesn't have the full PK specified? we should be careful about changing that)

[yashmayya]

Thanks for picking this up. The root cause in the description is correct. Calcite reads dim_tbl.currency = 'gbp' as
a non-equi condition, so the lookup key is shorter than the dimension table primary key.

I looked at the change closely and found some problems. I list them here for the record.

1. A constant replaces an equi-join key. The literal pass runs after the equi-join key pass, and it overwrites
without a condition:

_keyColumnLeftIds[pkPos] = -1;   // drops the equi-join key

Take ON dim.currency = fact.currency AND dim.rate_start_date = fact.rate_start_date AND dim.currency = 'gbp'. The
first position of the key becomes the constant gbp for every fact row. A fact row with usd then reads the gbp
dimension row. The equi-join key is not in the non-equi conditions, so no filter removes that row. The result is wrong
rows, not missing rows. I reproduced this order of the two passes in a test and got 4 rows where 2 are correct.

2. The literal keeps the type that the planner gave it. PrimaryKey compares values with equals, where an
Integer never equals a Long. A literal of the wrong numeric width misses every row. The value needs a conversion to
the stored type of the dimension column.

3. An open primary key column gives no error. When no condition fills a position, the key holds a null there and
the query returns 0 rows in silence.

4. A join key on a column outside the primary key is now dropped. The equi-join key pass skips a column that is
absent from the primary key. That condition is also absent from the non-equi conditions, so no filter applies it. The
old code returned 0 rows for this query, and the change returns rows that do not match the condition. The single-stage
lookup transform function rejects this case, and the multi-stage lookup join can do the same.

5. The change breaks every existing lookup join test.
ResourceBasedQueriesTest.registerMockDimensionTable mocks DimensionTableDataManager and does not stub
getPrimaryKeyColumns(). Mockito returns an empty list, so _keyColumnCount is 0 and every lookup key is empty. This
is why "Pinot Unit Test Set 1" fails on this branch.

6. The new test cannot pass, even after a correct fix. QueryRunnerTestBase.toRow puts the raw JSON values into
the row with no conversion to the type of the column. The fact side reads real segments, so it gets the type of the
column. The test declares rate_start_date as LONG, so the mock map holds an Integer while the query supplies a
Long. The mock needs the same conversion as a real dimension table.

7. Two smaller points. The file loses its last newline, which the linter reports. There is also no test for the
order fault. The same two conditions in reverse primary key order give 0 rows, because the key positions follow the
join condition instead of the primary key.

I opened #19210, which covers these cases and corrects the two faults in the test harness. It supersedes
this pull request.