gortiz commented on code in PR #20092:
URL: https://github.com/apache/pinot/pull/20092#discussion_r4423288388
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LookupJoinOperator.java:
##########
@@ -137,13 +140,43 @@ public LookupJoinOperator(OpChainExecutionContext
context, MultiStageOperator le
KeyPlan keyPlan =
compileKeyPlan(node, rightTableName,
_rightTable.getPrimaryKeyColumns(), _rightInput.getDataSchema(),
- _leftColumnSize);
+ _dimensionColumns, _leftColumnSize);
_keySize = keyPlan._sources.length;
_keySources = keyPlan._sources;
_keyConstants = keyPlan._constants;
_neverMatches = keyPlan._neverMatches;
}
+ /// Returns the dimension table column that each column of the right input
reads.
+ ///
+ /// The planner restricts the right input of a lookup join to `[Project ->]
TableScan`, and the Project holds input
+ /// references only. The output names of that Project are not table columns
when the query renames a column
+ /// (`SELECT id AS k`) or projects one twice (the planner then makes the
second name unique). The dimension table is
+ /// read by column name, so this method follows each Project column to the
TableScan column that it references. Using
+ /// the output names instead fails when a renamed column is a key column,
and returns nulls or the values of another
+ /// column otherwise.
+ @VisibleForTesting
+ static String[] resolveDimensionColumns(PlanNode rightInput, String
tableName) {
+ if (rightInput instanceof TableScanNode) {
+ return ((TableScanNode) rightInput).getColumns().toArray(new String[0]);
+ }
+ Preconditions.checkState(rightInput instanceof ProjectNode &&
rightInput.getInputs().size() == 1
+ && rightInput.getInputs().get(0) instanceof TableScanNode,
+ "Right input of lookup join on dimension table: %s must be a Project
over a TableScan, got: %s", tableName,
+ rightInput.explain());
+ List<String> scanColumns = ((TableScanNode)
rightInput.getInputs().get(0)).getColumns();
+ List<RexExpression> projects = ((ProjectNode) rightInput).getProjects();
+ String[] columns = new String[projects.size()];
+ for (int i = 0; i < columns.length; i++) {
+ RexExpression project = projects.get(i);
+ Preconditions.checkState(project instanceof RexExpression.InputRef,
+ "Right input of lookup join on dimension table: %s must project
columns only, got: %s at position: %s",
Review Comment:
Good catch, thanks. You are right that the v2 planner only checks the depth
of the right side, so a computed column reaches this check at execution time.
The new LookupJoin.json case accepts both errors for that reason. I will leave
the planner-side check out of this PR. The v2 planner does not get new
validation for now, and the runtime check already turns the silent nulls into
an error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
