leaves12138 commented on code in PR #7933: URL: https://github.com/apache/paimon/pull/7933#discussion_r3323143785
########## paimon-core/src/main/java/org/apache/paimon/globalindex/GlobalIndexBuilderUtils.java: ########## @@ -24,18 +24,31 @@ import org.apache.paimon.index.GlobalIndexMeta; import org.apache.paimon.index.IndexFileMeta; import org.apache.paimon.index.IndexPathFactory; +import org.apache.paimon.manifest.ManifestEntry; import org.apache.paimon.options.Options; +import org.apache.paimon.schema.SchemaManager; import org.apache.paimon.table.FileStoreTable; import org.apache.paimon.types.DataField; import org.apache.paimon.utils.Range; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; + import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; Review Comment: **Risk — `IndexManifestFileHandler` overlap detection false positive with multi-column indexes** `IndexManifestFileHandler.java:243` uses `indexFieldId` to determine whether two index files belong to the same field: ```java retainedMeta.indexFieldId() != addedMeta.indexFieldId() ``` With `MULTI_COLUMN_INDEX_FIELD_ID = -1`, **all** multi-column indexes share the same sentinel value. Two indexes on different column sets (e.g. `[title, vec]` vs `[content, embedding]`) will both have `indexFieldId == -1`, so the handler treats them as "same field". If their row ranges overlap, it throws `IllegalStateException` and rejects the commit — even though they are logically independent indexes. Suggestion: add `extraFieldIds` comparison (e.g. `Arrays.equals`) to the overlap check, or compare `indexType` as well. ########## paimon-core/src/main/java/org/apache/paimon/globalindex/GlobalIndexBuilderUtils.java: ########## @@ -24,18 +24,31 @@ import org.apache.paimon.index.GlobalIndexMeta; import org.apache.paimon.index.IndexFileMeta; import org.apache.paimon.index.IndexPathFactory; +import org.apache.paimon.manifest.ManifestEntry; import org.apache.paimon.options.Options; +import org.apache.paimon.schema.SchemaManager; import org.apache.paimon.table.FileStoreTable; import org.apache.paimon.types.DataField; import org.apache.paimon.utils.Range; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; + import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; Review Comment: **Minor — `TableIndexesTable` shows null for multi-column index field name** `TableIndexesTable.java:238` does `logicalRowType.getField(globalMeta.indexFieldId())` which throws when `indexFieldId == -1`. The exception is caught, but `index_field_name` silently displays `null` to users. Suggestion: when `indexFieldId == MULTI_COLUMN_INDEX_FIELD_ID`, resolve names from `extraFieldIds()` and join them with commas. -- 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]
