add optional 'castToType' parameter to 'auto' column schema #15417
Conversation
github-actions
bot
added
Area - Batch Ingestion
Area - Querying
Area - MSQ
| @@ -107,7 +107,7 @@ public Object getObject() | |||
| if (expressionType == null) { | |||
| return delegateDimensionSelector.getObject(); | |||
| } | |||
| return ExprEval.ofType(expressionType, delegateDimensionSelector.getObject()).value(); | |||
| return ExprEval.bestEffortOf(delegateDimensionSelector.getObject()).castTo(expressionType).value(); | |||
There was a problem hiding this comment.
whats the reason behind this change?
There was a problem hiding this comment.
this is a bug i noticed while testing some stuff with MSQ, ofType assumes that the typing information is absolutely correct and so will do what you tell it, which ends up doing some not great things. For example, without bestEffort/explicit cast, ingesting some column specifieid as ARRAY<STRING> in an extern in MSQ that has some rows with arrays of nested objects will do something like this
which should instead be a parse exception
| @@ -73,7 +73,7 @@ public static DimensionSchema createDimensionSchema( | |||
| .getDimensionSchema(capabilities); | |||
| } | |||
|
|
|||
| return new AutoTypeColumnSchema(column); | |||
| return new AutoTypeColumnSchema(column, type != null && type.is(ValueType.COMPLEX) ? type : null); | |||
There was a problem hiding this comment.
shouldn't this be
type != null && !type.is(ValueType.COMPLEX)
There was a problem hiding this comment.
since you can't cast to complex types.
There was a problem hiding this comment.
oops yea, or should just leave it null
| return new EncodedKeyComponent<>(StructuredData.wrap(eval.value()), effectiveSizeBytes); | ||
| } | ||
| catch (IAE invalidCast) { | ||
| throw new ParseException(eval.asString(), invalidCast, "Cannot coerce to requested type [%s]", castToType); |
There was a problem hiding this comment.
can we include the field / column name here?
There was a problem hiding this comment.
seems reasonable, but will make sure whatever catches these doesn't already do something like that
processing/src/main/java/org/apache/druid/segment/AutoTypeColumnIndexer.java
Show resolved
Hide resolved
| @@ -148,20 +152,26 @@ public void writeMergedValueDictionary(List<IndexableAdapter> adapters) throws I | |||
| final FieldTypeInfo.MutableTypeSet rootTypes = mergedFields.get(NestedPathFinder.JSON_PATH_ROOT); | |||
| final boolean rootOnly = mergedFields.size() == 1 && rootTypes != null; | |||
|
|
|||
| final ColumnType explicitType; | |||
| if (castToType != null && (castToType.isPrimitive() || castToType.isPrimitiveArray())) { | |||
There was a problem hiding this comment.
do you actually need the (castToType.isPrimitive() || castToType.isPrimitiveArray()) check here or is it just defensive coding?
processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndex.java
Show resolved
Hide resolved
…5417) * auto but.. with an expected type
…5417) * auto but.. with an expected type
Description
This PR updates the 'auto' schema to add a new
castToTypeparameter which accepts aColumnTypeto allow forcing the 'auto' indexer to cast values to a specific type. One of the main use cases for this is MSQ array ingestion, which can now specify the exact desired type, but also allows known type columns to use all of the new features provided by 'auto' columns such as numeric indexes and array type columns for any other native ingestion. This only works for primitive/primitive array types, and will be ignored if any other type is used. The column part serde stores an extraenforceLogicalTypewhich is used to ensure the correct casting schema is created for stuff like auto-compaction.Release note
tbd
This PR has: