FrankChen021 commented on code in PR #20377:
URL: https://github.com/apache/druid/pull/20377#discussion_r4071598265
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/ProjectionSpecTranslator.java:
##########
@@ -265,23 +350,9 @@ private static VirtualColumns liftComputedColumns(
// A plain reference: the column is ingested as it arrives.
continue;
}
- if (!(virtualColumn instanceof ExpressionVirtualColumn)) {
- throw invalid(
- BASE_PROJECTION_NAME,
- "column [" + declared + "] is computed by an expression the base
table cannot store"
- );
- }
- final ExpressionVirtualColumn expression = (ExpressionVirtualColumn)
virtualColumn;
- materialized.add(
- new ExpressionVirtualColumn(
- declared,
- expression.getExpression(),
- expression.getOutputType(),
- ExprMacroTable.nil()
- )
- );
+ computed.add(new ComputedColumn(declared, virtualColumn));
Review Comment:
P2 Specialized computed columns lose dependencies
**Finding:** When a clustered __base DDL expression is composed from
planner-specialized virtual columns, liftComputedColumns records only the
selected output virtual column. The ScanQuery can contain intermediary virtual
columns recursively (for example, a nested JSON expression can produce an outer
NestedFieldVirtualColumn that reads a NestedObjectVirtualColumn which reads an
inner NestedFieldVirtualColumn), but those dependencies are not copied into the
materialized base-table spec.
ClusteredValueGroupsBaseTableProjectionSpec.validateVirtualColumns then sees
the renamed output still reading synthetic vN names that are neither stored
columns nor virtual columns, so valid composed expressions are rejected during
DDL translation.
**Suggestion:** Retain the full dependency closure from the planned
ScanQuery when lifting a computed output, keeping intermediary virtual columns
under their synthetic names while renaming only the materialized root (or
rewrite the root into a self-contained expression). Add a clustered base-table
DDL test for a nested specialized expression composition.
--
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]
