github-actions[bot] commented on code in PR #69761:
URL: https://github.com/apache/doris/pull/69761#discussion_r4492769852


##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -3357,6 +3382,15 @@ private boolean processAddIndex(CreateIndexOp 
createIndexOp, OlapTable olapTable
                     + " recreate the table with inverted_index_storage_format 
= V2.");
         }
 
+        if (indexDef.isVariantValuesIndex() && Config.isCloudMode()) {
+            if (olapTable.getPartitionInvertedIndexFileStorageFormat() != 
TInvertedIndexFileStorageFormat.SNII
+                    || olapTable.getPartitions().stream().anyMatch(partition ->
+                            
olapTable.getInvertedIndexFileStorageFormatForPartition(partition.getId())

Review Comment:
   [P1] Preserve the SNII invariant when recovering partitions
   
   With cloud partition-format rollout enabled, a base-SNII table without a 
values index can create a V2 partition, drop it without FORCE, switch the 
future format back to SNII, and pass this new-data-only values-index ADD check. 
`RECOVER PARTITION ... FROM t` then succeeds: `CatalogRecycleBin` compares only 
materialized-index IDs (unchanged by light ADD) and restores the saved V2 
format. Subsequent inserts combine current values-index metadata with the 
recovered tablet's V2 writer. `IndexFileWriter::add_snii_index` hits its SNII 
`DCHECK`; with assertions disabled, the V2 close path never seals the SNII 
compound index. Enforce the invariant before recovery publishes the partition 
and add this lifecycle test. This is distinct from the fixed SET/ADD race: no 
concurrent statement is needed.



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java:
##########
@@ -562,6 +575,66 @@ public Map<String, String> getProperties() {
         return properties;
     }
 
+    public boolean isVariantValuesIndex() {
+        return indexType == IndexType.INVERTED && 
InvertedIndexProperties.isVariantValuesIndex(properties);
+    }
+
+    private void validateVariantValuesIndex(DataType columnType,
+            TInvertedIndexFileStorageFormat invertedIndexFileStorageFormat) {
+        String scope = 
properties.get(InvertedIndexUtil.VARIANT_INDEX_SCOPE_KEY);
+        if (scope == null) {
+            return;
+        }
+        if (!isVariantValuesIndex()) {
+            throw new AnalysisException("variant_index_scope must be values, 
found: " + scope);
+        }
+        if (!columnType.isVariantType()) {
+            throw new AnalysisException(
+                    "variant_index_scope=values can only be used on VARIANT 
columns");
+        }
+        // Declared Decimal256 can be stored, but full-root assembly cannot 
decode it.
+        // Reject this combination before accepting a table whose legal 
inserts would fail.
+        VariantType variantType = (VariantType) columnType;
+        if (variantType.getEnableNestedGroup()) {
+            throw new AnalysisException("VARIANT values index does not support 
NestedGroup");
+        }
+        if (variantType.getPredefinedFields().stream()
+                .anyMatch(field -> containsDecimal256(field.getDataType()))) {
+            throw new AnalysisException("VARIANT values index does not support 
declared Decimal256 paths");
+        }
+        if (invertedIndexFileStorageFormat != 
TInvertedIndexFileStorageFormat.SNII) {

Review Comment:
   [P1] Enforce values-index compatibility during RESTORE
   
   With experimental cloud RESTORE enabled, a local V2 backup with ordinary 
`idx(v)` can restore an absent partition into an existing SNII table with 
matching columns and a same-named values `idx(v)`. `OlapTable.getSignature()` 
compares index names/types/columns but omits scope and physical format. For a 
case without shared-schema-key collisions, back up a replacement tablet after 
heavy `aux INT -> BIGINT`, and create the destination directly with the 
matching final BIGINT column; source schema version 1 differs from destination 
version 0. Although `CloudRestoreJob` initially creates SNII tablets, 
`CloudSnapshotMgr` copies source tablet metadata and MS `commit_restore_job` 
overwrites the target metadata with it. `commit_snapshot` then evicts the 
tablet cache. The next INSERT reloads V2 while receiving current values-index 
metadata, reaching the SNII assertion/unsealed-index path. Validate 
scope/format during RESTORE and preserve this invariant through its durable 
commit; checking DDL
  or initial tablet creation alone does not cover it.



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