github-actions[bot] commented on code in PR #64680:
URL: https://github.com/apache/doris/pull/64680#discussion_r3459565404
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -5329,8 +5329,11 @@ public DataType
visitVariantPredefinedFields(VariantPredefinedFieldsContext ctx)
}
if (enableNestedGroup) {
- throw new NotSupportedException(
- "variant_enable_nested_group is not supported now");
+ enableVariantDocMode = false;
+ variantMaxSubcolumnsCount = 0;
Review Comment:
This normalization should validate the final VARIANT property combination
after session defaults and explicit properties have both been applied. Right
now the validator only sees the raw property map, so conflicts where one side
comes from a default are silently rewritten here. For example, with
`default_variant_enable_nested_group=true`, a column declared as
`VARIANT<PROPERTIES("variant_enable_doc_mode"="true")>` reaches this branch
with both booleans true and the explicit doc-mode request is forced back to
false; with `default_variant_enable_doc_mode=true`, an explicit nested-group
property does the same. A positive explicit `variant_max_subcolumns_count` is
also zeroed when nested group comes from the session default. Please validate
`enableNestedGroup` against the final doc/sparse/subcolumn settings before
normalizing them, and add parser tests that cover the session-default cases.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -5329,8 +5329,11 @@ public DataType
visitVariantPredefinedFields(VariantPredefinedFieldsContext ctx)
}
if (enableNestedGroup) {
- throw new NotSupportedException(
- "variant_enable_nested_group is not supported now");
+ enableVariantDocMode = false;
Review Comment:
Now that `variant_enable_nested_group=true` can be parsed into a Nereids
`VariantType`, explicit casts to that type need to remain semantically visible.
Today `SimplifyCastRule` removes `CAST(v AS
VARIANT<PROPERTIES("variant_enable_nested_group"="true")>)` whenever the child
is any otherwise-equal VARIANT, because `VariantType.equals()` and `hashCode()`
do not include `enableNestedGroup`. In CTAS/schema-from-query paths, the output
column type is taken from the rewritten slot via
`s.getDataType().conversion()`, so the cast can be optimized away and the
created table loses the requested nested-group property. Please either include
`enableNestedGroup` in Nereids `VariantType` equality/hash semantics, or
otherwise prevent cast simplification from dropping explicit VARIANT property
casts, and add a test that covers this through CTAS or output schema derivation.
--
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]
