github-actions[bot] commented on code in PR #63147:
URL: https://github.com/apache/doris/pull/63147#discussion_r3221495745
##########
fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java:
##########
@@ -723,9 +724,100 @@ public void checkSchemaChangeAllowed(Column other) throws
DdlException {
if (this.getVariantEnableNestedGroup() !=
other.getVariantEnableNestedGroup()) {
throw new DdlException("Can not change variant enable nested
group");
}
- if (CollectionUtils.isNotEmpty(this.getChildren()) ||
CollectionUtils.isNotEmpty(other.getChildren())) {
- throw new DdlException("Can not change variant schema
templates");
+ if (hasVariantSchemaTemplateChange(other)
+ && this.getvariantDocMaterializationMinRows() !=
other.getvariantDocMaterializationMinRows()) {
+ throw new DdlException("Can not change variant doc snapshot
materialization min rows "
+ + "when changing variant schema templates");
}
+ checkVariantSchemaTemplateChange(other);
+ }
+ }
+
+ private boolean hasVariantSchemaTemplateChange(Column other) {
+ ArrayList<VariantField> oldFields = ((VariantType)
type).getPredefinedFields();
+ ArrayList<VariantField> newFields = ((VariantType)
other.type).getPredefinedFields();
+ if (oldFields.size() != newFields.size()) {
+ return true;
+ }
+ for (int i = 0; i < oldFields.size(); i++) {
+ VariantField oldField = oldFields.get(i);
+ VariantField newField = newFields.get(i);
+ if (!Objects.equals(oldField.getPattern(), newField.getPattern())
+ || !Objects.equals(oldField.getPatternType(),
newField.getPatternType())
Review Comment:
These new pattern-type/comment checks are bypassed for the exact cases they
try to reject. `SchemaChangeHandler` only calls
`oriColumn.checkSchemaChangeAllowed(alterColumn)` when
`!alterColumn.equals(oriColumn)`, but `Column.equals()` delegates to
`VariantType.equals()`, whose `VariantField.equals()` ignores both
`patternType` and `comment` (and the child `Column` equality ignores
`fieldPatternType`). As a result, `ALTER TABLE ... MODIFY COLUMN v
variant<MATCH_NAME 'a' : string>` to `variant<'a' : string>` or changing the
template comment is accepted and overwrites the metadata without throwing these
errors. Please include these attributes in equality/change detection or invoke
the variant schema-template validation before the equality short-circuit, and
add a test for a pattern-type-only alter.
--
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]
