FrankChen021 commented on code in PR #19842:
URL: https://github.com/apache/druid/pull/19842#discussion_r3695810847


##########
server/src/main/java/org/apache/druid/catalog/model/ClusteredValueGroupsBaseTableMetadata.java:
##########
@@ -212,16 +215,54 @@ private DimensionSchema toDimensionSchema(ColumnSpec 
column, @Nullable Dimension
     if (ColumnType.NESTED_DATA.equals(druidType)) {
       return new NestedDataColumnSchema(column.name(), 
NestedDataColumnSchema.DEFAULT_FORMAT_VERSION);
     }
-    // Other complex types cannot be ingested into a clustered base table: 
there is no dimension handler for them,
-    // and clustered base tables have no aggregators to produce them.
+    if (druidType.is(ValueType.COMPLEX)) {
+      return complexDimensionSchema(column.name(), druidType);
+    }
     throw InvalidInput.exception(
-        "column [%s] has unsupported type [%s] for a clustered base table; 
supported types are primitive, primitive"
-        + " array, and COMPLEX<json> columns",
+        "column [%s] has unsupported type [%s] for a clustered base table",
         column.name(),
         druidType
     );
   }
 
+  /**
+   * Resolve a complex column through its registered {@link 
org.apache.druid.segment.DimensionHandler}, so that any
+   * complex type which can be stored as a dimension may be declared, 
including types contributed by extensions. The
+   * handler is looked up by the complex type name, so the schema it returns 
is specific to the declared type.
+   * <p>
+   * The returned schema is checked against the declared type before it is 
accepted. A schema selects its own handler
+   * at ingest time (via {@link DimensionSchema#getDimensionHandler()}, which 
reads
+   * {@link DimensionSchema#getColumnType()}), so a schema of some other type 
would quietly store the column as that
+   * type instead, contradicting the declared schema that queries are 
validated and coerced against.
+   */
+  private static DimensionSchema complexDimensionSchema(String name, 
ColumnType druidType)
+  {
+    final DimensionSchema schema;
+    try {
+      schema = DimensionHandlerUtils.getComplexDimensionSchema(name, 
druidType);
+    }
+    catch (ISE e) {
+      // No handler is registered for this complex type, which usually means 
the extension that defines it is not
+      // loaded on whichever service is validating the spec.
+      throw InvalidInput.exception(
+          "column [%s] has type [%s], which cannot be stored as a dimension of 
a clustered base table; if this type"
+          + " comes from an extension, check that the extension is loaded",
+          name,
+          druidType
+      );
+    }
+    if (!druidType.equals(schema.getColumnType())) {

Review Comment:
   [P2] Reject handler schemas that rename the column
   
   The new guard validates only the returned schema's type. A provider can 
still return the correct complex type under a different column name, which 
createSpec accepts verbatim. Downstream clustered ingestion then reads 
row.getRaw(schema.getName()), silently storing nulls for the declared catalog 
column and exposing the provider-chosen name instead. Validate 
name.equals(schema.getName()) alongside the type.



-- 
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]

Reply via email to