eldenmoon commented on code in PR #69761:
URL: https://github.com/apache/doris/pull/69761#discussion_r4500358543
##########
be/src/storage/compaction/compaction.cpp:
##########
@@ -1663,7 +1678,8 @@ void
Compaction::construct_index_compaction_columns(RowsetWriterContext& ctx) {
}
const int32_t col_unique_id = col_unique_ids[0];
if (!_cur_tablet_schema->has_column_unique_id(col_unique_id) ||
-
!field_is_slice_type(_cur_tablet_schema->column_by_uid(col_unique_id).type())) {
+
(!field_is_slice_type(_cur_tablet_schema->column_by_uid(col_unique_id).type())
&&
+ !destination_index->is_variant_values_index())) {
Review Comment:
Unchanged: accepted limitation, see the earlier reply; the type promotion
will be fixed separately in VARIANT.
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -3471,6 +3481,10 @@ private boolean processDropIndex(DropIndexOp
dropIndexOp, OlapTable olapTable, L
throw new DdlException("index " + indexName + " does not exist");
}
+ if (found.isVariantValuesIndex()) {
+ throw new DdlException("DROP INDEX is not supported for a VARIANT
values index");
Review Comment:
Fixed in earlier commits and confirmed by the review of 8c1bf672753: full
values-index DROP follows the supported deletion path (90951f6117d), the cloud
SET/ADD race is rechecked under the table write lock and MOR ADD/CREATE INDEX
rejects analyzer properties like CREATE TABLE (e0af3d9d70a).
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/alter/CloudSchemaChangeHandler.java:
##########
@@ -158,6 +159,12 @@ public void updateTableProperties(Database db, String
tableName, Map<String, Str
}
olapTable.readLock();
try {
+ // New partitions must retain the format required by the
table's values indexes.
+ if (invertedIndexFileStorageFormat !=
TInvertedIndexFileStorageFormat.SNII
Review Comment:
Fixed in earlier commits and confirmed by the review of 8c1bf672753: full
values-index DROP follows the supported deletion path (90951f6117d), the cloud
SET/ADD race is rechecked under the table write lock and MOR ADD/CREATE INDEX
rejects analyzer properties like CREATE TABLE (e0af3d9d70a).
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/IndexDefinition.java:
##########
@@ -380,6 +385,8 @@ public void checkColumn(Column column, KeysType keysType,
boolean enableUniqueKe
}
if (indexType == IndexType.INVERTED) {
+
validateVariantValuesIndex(DataType.fromCatalogType(column.getType()),
Review Comment:
Fixed in earlier commits and confirmed by the review of 8c1bf672753: full
values-index DROP follows the supported deletion path (90951f6117d), the cloud
SET/ADD race is rechecked under the table write lock and MOR ADD/CREATE INDEX
rejects analyzer properties like CREATE TABLE (e0af3d9d70a).
##########
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:
Fixed in 3b12c163494 by a BE rule instead of per-path FE checks: a values
index is stored only in SNII index files. A segment whose index file is V2/V3
is written without the values index (`VariantV2ColumnWriter::init`), and
reading it registers no values-index reader (`ColumnReader::_load_index`), so
MATCH evaluates rows, exactly as for segments written before a new-data-only
ADD. A RECOVERed or RESTOREd legacy partition, including the cloud path where
MS copies the backup's tablet meta, therefore writes and reads correctly
without the index; it no longer reaches the SNII writer. The FE lifecycle
checks from 4000def679a are reverted. `variant_values_index_test` covers
V2/V3/SNII writes, reads and full compaction.
--
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]
