clintropolis commented on code in PR #15920:
URL: https://github.com/apache/druid/pull/15920#discussion_r1523815715


##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/DimensionSchemaUtils.java:
##########
@@ -57,76 +57,112 @@ public static DimensionSchema 
createDimensionSchemaForExtern(final String column
     );
   }
 
+  /**
+   * Create a dimension schema for a dimension column, given the type that it 
was assigned in the query, and the
+   * current values of {@link MultiStageQueryContext#CTX_USE_AUTO_SCHEMAS} and
+   * {@link MultiStageQueryContext#CTX_ARRAY_INGEST_MODE}.
+   *
+   * @param column          column name
+   * @param queryType       type of the column from the query
+   * @param useAutoType     active value of {@link 
MultiStageQueryContext#CTX_USE_AUTO_SCHEMAS}
+   * @param arrayIngestMode active value of {@link 
MultiStageQueryContext#CTX_ARRAY_INGEST_MODE}
+   */
   public static DimensionSchema createDimensionSchema(
       final String column,
-      @Nullable final ColumnType type,
+      @Nullable final ColumnType queryType,
       boolean useAutoType,
       ArrayIngestMode arrayIngestMode
   )
   {
     if (useAutoType) {
       // for complex types that are not COMPLEX<json>, we still want to use 
the handler since 'auto' typing
       // only works for the 'standard' built-in types
-      if (type != null && type.is(ValueType.COMPLEX) && 
!ColumnType.NESTED_DATA.equals(type)) {
-        final ColumnCapabilities capabilities = 
ColumnCapabilitiesImpl.createDefault().setType(type);
+      if (queryType != null && queryType.is(ValueType.COMPLEX) && 
!ColumnType.NESTED_DATA.equals(queryType)) {
+        final ColumnCapabilities capabilities = 
ColumnCapabilitiesImpl.createDefault().setType(queryType);
         return DimensionHandlerUtils.getHandlerFromCapabilities(column, 
capabilities, null)
                                     .getDimensionSchema(capabilities);
       }
 
-      if (type != null && (type.isPrimitive() || type.isPrimitiveArray())) {
-        return new AutoTypeColumnSchema(column, type);
+      if (queryType != null && (queryType.isPrimitive() || 
queryType.isPrimitiveArray())) {
+        return new AutoTypeColumnSchema(column, queryType);
       }
       return new AutoTypeColumnSchema(column, null);
     } else {
-      // if schema information is not available, create a string dimension
-      if (type == null) {
-        return new StringDimensionSchema(column);
-      } else if (type.getType() == ValueType.STRING) {
-        return new StringDimensionSchema(column);
-      } else if (type.getType() == ValueType.LONG) {
+      // dimensionType may not be identical to queryType, depending on 
arrayIngestMode.
+      final ColumnType dimensionType = getDimensionType(queryType, 
arrayIngestMode);
+
+      if (dimensionType.getType() == ValueType.STRING) {
+        return new StringDimensionSchema(
+            column,
+            queryType != null && queryType.isArray()
+            ? DimensionSchema.MultiValueHandling.ARRAY
+            : DimensionSchema.MultiValueHandling.SORTED_ARRAY,
+            null
+        );
+      } else if (dimensionType.getType() == ValueType.LONG) {
         return new LongDimensionSchema(column);
-      } else if (type.getType() == ValueType.FLOAT) {
+      } else if (dimensionType.getType() == ValueType.FLOAT) {
         return new FloatDimensionSchema(column);
-      } else if (type.getType() == ValueType.DOUBLE) {
+      } else if (dimensionType.getType() == ValueType.DOUBLE) {
         return new DoubleDimensionSchema(column);
-      } else if (type.getType() == ValueType.ARRAY) {
-        ValueType elementType = type.getElementType().getType();
-        if (elementType == ValueType.STRING) {
-          if (arrayIngestMode == ArrayIngestMode.NONE) {
-            throw InvalidInput.exception(
-                "String arrays can not be ingested when '%s' is set to '%s'. 
Set '%s' in query context "
-                + "to 'array' to ingest the string array as an array, or 
ingest it as an MVD by explicitly casting the "
-                + "array to an MVD with ARRAY_TO_MV function.",
-                MultiStageQueryContext.CTX_ARRAY_INGEST_MODE,
-                StringUtils.toLowerCase(arrayIngestMode.name()),
-                MultiStageQueryContext.CTX_ARRAY_INGEST_MODE
-            );
-          } else if (arrayIngestMode == ArrayIngestMode.MVD) {
-            return new StringDimensionSchema(column, 
DimensionSchema.MultiValueHandling.ARRAY, null);
-          } else {
-            // arrayIngestMode == ArrayIngestMode.ARRAY would be true
-            return new AutoTypeColumnSchema(column, type);
-          }
-        } else if (elementType.isNumeric()) {
-          // ValueType == LONG || ValueType == FLOAT || ValueType == DOUBLE
-          if (arrayIngestMode == ArrayIngestMode.ARRAY) {
-            return new AutoTypeColumnSchema(column, type);
-          } else {
-            throw InvalidInput.exception(
-                "Numeric arrays can only be ingested when '%s' is set to 
'array' in the MSQ query's context. "
-                + "Current value of the parameter [%s]",
-                MultiStageQueryContext.CTX_ARRAY_INGEST_MODE,
-                StringUtils.toLowerCase(arrayIngestMode.name())
-            );
-          }
-        } else {
-          throw new ISE("Cannot create dimension for type [%s]", 
type.toString());
-        }
+      } else if (dimensionType.getType() == ValueType.ARRAY) {
+        return new AutoTypeColumnSchema(column, dimensionType);
       } else {
-        final ColumnCapabilities capabilities = 
ColumnCapabilitiesImpl.createDefault().setType(type);
+        final ColumnCapabilities capabilities = 
ColumnCapabilitiesImpl.createDefault().setType(dimensionType);
         return DimensionHandlerUtils.getHandlerFromCapabilities(column, 
capabilities, null)
                                     .getDimensionSchema(capabilities);
       }
     }
   }
+
+  /**
+   * Based on a type from a query result, get the type of dimension we should 
write.
+   *
+   * @throws org.apache.druid.error.DruidException if there is some problem
+   */
+  public static ColumnType getDimensionType(
+      @Nullable final ColumnType queryType,
+      final ArrayIngestMode arrayIngestMode
+  )
+  {
+    if (queryType == null) {
+      // if schema information is not available, create a string dimension
+      return ColumnType.STRING;

Review Comment:
   ah, I should have looked at the callers, that makes sense, leaving for now 
and making an error later seems fine to me



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