xiangfu0 commented on code in PR #20092:
URL: https://github.com/apache/pinot/pull/20092#discussion_r4415546808
##########
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:
Minor, non-blocking: in the v2 planner this check is the only guard.
`PlanFragmentAndMailboxAssignment.processLookupLocalExchange` checks depth
only, so `SELECT id, UPPER(a) AS u` gets through planning and fails here at
execution time as `QUERY_EXECUTION`. v1 rejects the same query at planning
time. On master the query returned nulls with no error, so failing is the right
outcome. A follow-up could add the same `RexInputRef`-only check to the v2
planner so both planners fail the same way, before dispatch.
--
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]
