FrankChen021 commented on code in PR #19845:
URL: https://github.com/apache/druid/pull/19845#discussion_r3783671143
##########
multi-stage-query/src/main/java/org/apache/druid/msq/indexing/destination/SegmentGenerationUtils.java:
##########
@@ -212,6 +220,59 @@ private static boolean isRollupQuery(Query<?> query)
&&
!query.context().getBoolean(GroupByQueryConfig.CTX_KEY_ENABLE_MULTI_VALUE_UNNESTING,
true);
}
+ /**
+ * The columns a query produces that a base table does not declare, in query
output order, as the
+ * {@link DimensionSchema}s they should be stored with.
+ * <p>
+ * A base table declares the columns an operator asked for. When the target
table does not require a strict schema,
+ * a query may produce others; they are stored so that ingesting a column is
never silently a no-op.
+ */
+ private static List<DimensionSchema> undeclaredColumns(
+ final BaseTableProjectionSpec baseTable,
+ final RowSignature querySignature,
+ final ColumnMappings columnMappings,
+ final Query<?> query,
+ @Nullable final Map<String, DimensionSchema> dimensionSchemas
+ )
+ {
+ final Set<String> declared = new HashSet<>();
+ for (DimensionSchema dimension :
baseTable.getDimensionsSpec().getDimensions()) {
+ declared.add(dimension.getName());
+ }
+ for (AggregatorFactory metric : baseTable.getMetrics() == null ? new
AggregatorFactory[0] : baseTable.getMetrics()) {
+ declared.add(metric.getName());
+ }
+ // The time column is positional in a base table, never appended.
+ declared.add(ColumnHolder.TIME_COLUMN_NAME);
+
+ final List<DimensionSchema> undeclared = new ArrayList<>();
+ for (final String outputColumnName :
columnMappings.getOutputColumnNames()) {
+ if (!declared.add(outputColumnName)) {
Review Comment:
[P2] Do not append intermediate virtual columns as writable fields
Virtual-column outputs that are intermediate rather than declared dimensions
are omitted from `declared`, so an unsealed insert can append such a name as a
writable dimension. Virtual-column evaluation then overrides the query-supplied
value for that field. Reject virtual output names before appending them to the
writable schema.
##########
multi-stage-query/src/main/java/org/apache/druid/msq/indexing/destination/SegmentGenerationUtils.java:
##########
@@ -212,6 +220,59 @@ private static boolean isRollupQuery(Query<?> query)
&&
!query.context().getBoolean(GroupByQueryConfig.CTX_KEY_ENABLE_MULTI_VALUE_UNNESTING,
true);
}
+ /**
+ * The columns a query produces that a base table does not declare, in query
output order, as the
+ * {@link DimensionSchema}s they should be stored with.
+ * <p>
+ * A base table declares the columns an operator asked for. When the target
table does not require a strict schema,
+ * a query may produce others; they are stored so that ingesting a column is
never silently a no-op.
+ */
+ private static List<DimensionSchema> undeclaredColumns(
+ final BaseTableProjectionSpec baseTable,
+ final RowSignature querySignature,
+ final ColumnMappings columnMappings,
+ final Query<?> query,
+ @Nullable final Map<String, DimensionSchema> dimensionSchemas
+ )
+ {
+ final Set<String> declared = new HashSet<>();
+ for (DimensionSchema dimension :
baseTable.getDimensionsSpec().getDimensions()) {
+ declared.add(dimension.getName());
+ }
+ for (AggregatorFactory metric : baseTable.getMetrics() == null ? new
AggregatorFactory[0] : baseTable.getMetrics()) {
+ declared.add(metric.getName());
+ }
+ // The time column is positional in a base table, never appended.
+ declared.add(ColumnHolder.TIME_COLUMN_NAME);
+
+ final List<DimensionSchema> undeclared = new ArrayList<>();
+ for (final String outputColumnName :
columnMappings.getOutputColumnNames()) {
+ if (!declared.add(outputColumnName)) {
+ continue;
+ }
+ final int outputColumn = CollectionUtils.getOnlyElement(
+ columnMappings.getOutputColumnsByName(outputColumnName),
+ xs -> new ISE("Expected single output column for name [%s], but got
[%s]", outputColumnName, xs)
+ );
+ final String queryColumn =
columnMappings.getQueryColumnName(outputColumn);
+ final ColumnType type =
+ querySignature.getColumnType(queryColumn)
+ .orElseThrow(() -> new ISE("No type for column [%s]",
outputColumnName));
+
+ if (type.is(ValueType.COMPLEX) &&
!DimensionHandlerUtils.DIMENSION_HANDLER_PROVIDERS.containsKey(type.getComplexTypeName()))
{
Review Comment:
[P2] Handle unknown complex types without an internal NPE
`ColumnType.UNKNOWN_COMPLEX` has a null complex type name, and passing that
name to `ConcurrentHashMap.containsKey` throws `NullPointerException` instead
of the intended user-facing `InvalidInput`. Guard null complex names before the
provider lookup so unknown complex types follow the documented validation path.
--
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]
