fix complex types returning UNKNOWN as their SQL type inference#16216
fix complex types returning UNKNOWN as their SQL type inference#16216zachjsh merged 4 commits intoapache:masterfrom
Conversation
| ); | ||
| } | ||
|
|
||
| public static SqlReturnTypeInference complexReturnTypeWithNullability(ColumnType columnType, boolean nullable) |
There was a problem hiding this comment.
nit: there are some SqlReturnTypeInference defined in Calcites, maybe this should also live there (or those should live here?)
| .operandTypes(SqlTypeFamily.ANY, SqlTypeFamily.INTEGER) | ||
| .returnTypeNonNull(SqlTypeName.OTHER) | ||
| .returnTypeInference( | ||
| OperatorConversions.complexReturnTypeWithNullability(SketchModule.BUILD_TYPE, false)) |
There was a problem hiding this comment.
i guess the changes to ComplexSqlType make this not really matter anymore, but technically THETA_SKETCH is the type string for what gets stored in columns and is associated with the serde that is common between build/merge (and the postagg the function maps to is actually using the merge type as its output type)
also, nit formatting, trailing ) should be on newline (though style stuff doesn't always flag this consistently). same formatting nit for other similar operator function changes
There was a problem hiding this comment.
Thanks fixed, and think I got all the formatting ones, let me know if I missed any.
| .operandTypes(SqlTypeFamily.ANY, SqlTypeFamily.EXACT_NUMERIC) | ||
| .returnTypeNonNull(SqlTypeName.OTHER) | ||
| .returnTypeInference( | ||
| OperatorConversions.complexReturnTypeWithNullability(HllSketchModule.TYPE, false)) |
There was a problem hiding this comment.
this should be DoublesSketchModule.DOUBLES_SKETCH i think?
| .operandTypes(SqlTypeFamily.ANY, SqlTypeFamily.INTEGER) | ||
| .requiredOperandCount(1) | ||
| .returnTypeNonNull(SqlTypeName.OTHER) | ||
| .returnTypeInference( |
There was a problem hiding this comment.
this function returns ARRAY<DOUBLE> as far as I can tell. I think there is a .returnTypeNullableArrayWithNullableElements that can be used instead of .returnTypeInference
| public static final SqlReturnTypeInference | ||
| ARG1_NULLABLE_ARRAY_RETURN_TYPE_INFERENCE = new Arg1NullableArrayTypeInference(); | ||
|
|
||
| public static SqlReturnTypeInference complexReturnTypeWithNullability(ColumnType columnType, boolean nullable) |
There was a problem hiding this comment.
Oh, I guess we could have defined this method on OperatorBuilder, similar to the others https://github.com/apache/druid/blob/master/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java#L366
4 participants
Description
Fixes issue where several aggregators are returning UNKNOWN or OTHER as their SQL type inference. Fixing this will allow these types do be validated against when defined as column on tables stored in the catalog.
This PR has: