You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "QuakeWang (via GitHub)" <gi...@apache.org> on 2026/08/26 06:36:18 UTC

[PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

QuakeWang opened a new pull request, #9402:
URL: https://github.com/apache/paimon/pull/9402

   ### Purpose
   
   Global indexes are written with serializers derived from the build schema, while readers use the current table schema. After an indexed field type evolves, reusing an old index can miss matches, and counting that index in coverage can incorrectly skip the required data scan.
   
   This change records the build schema ID in global-index metadata and centrally compares the indexed fields' logical types before reader grouping and coverage calculation. Legacy metadata without a build schema ID fails closed, and the new field is preserved across builders, serializers, and row-ID reassignment.
   
   ### Tests
   
     - Core targeted tests: 150 passed
     - Checkstyle, Spotless, Enforcer, and Apache RAT
     - Flink, Spark, full-text, vector, and lumina test compilation
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9402:
URL: https://github.com/apache/paimon/pull/9402#discussion_r3930444034


##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/copy/ListIndexFilesOperator.java:
##########
@@ -66,15 +70,27 @@ public List<CopyFileInfo> execute(
         FileStoreTable targetTable = (FileStoreTable) targetCatalog.getTable(targetIdentifier);
         List<CopyFileInfo> indexFiles = new ArrayList<>();
         IndexFileHandler sourceIndexHandler = sourceTable.store().newIndexFileHandler();
+        FileStorePathFactory sourceFileStorePathFactory = sourceTable.store().pathFactory();
         FileStorePathFactory targetFileStorePathFactory = targetTable.store().pathFactory();
         List<IndexManifestEntry> indexManifestEntries =
                 sourceIndexHandler.readManifestWithIOException(snapshot.indexManifest());
+        Set<IndexManifestEntry> compatibleGlobalIndexes =
+                new HashSet<>(
+                        GlobalIndexSchemaCompatibility.filterCompatible(
+                                sourceTable, indexManifestEntries));
         for (IndexManifestEntry indexManifestEntry : indexManifestEntries) {
+            boolean globalIndex = indexManifestEntry.indexFile().globalIndexMeta() != null;
+            if (globalIndex && !compatibleGlobalIndexes.contains(indexManifestEntry)) {

Review Comment:
   [P2] Preserve primary-key global-index payloads during copy
   
   This filter applies to every entry with `globalIndexMeta() != null`, but the production PK payload builders still create their `IndexFileMeta` without a schema id (`PkFullTextIndexFile`, `PkSortedIndexFile`, and `PkVectorAnnSegmentFile`). `ManifestEntryChanges` only propagates that null value, so `filterCompatible` rejects all of these payloads and this branch silently omits them from `copy_files`. Before this PR they were copied. The target data can generally fall back to raw/exact reads, but the copied table loses all PK sorted/full-text/vector acceleration, and FAST-mode behavior can change.
   
   Please either stamp the build schema id in those PK payload builders and validate them on read, or scope this compatibility filter to index families that actually carry schema identity. A copy test should assert each PK payload remains in the target manifest and produces equivalent queries.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "QuakeWang (via GitHub)" <gi...@apache.org>.
QuakeWang closed pull request #9402: [core] Validate global-index schema compatibility before reader and coverage
URL: https://github.com/apache/paimon/pull/9402


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9402:
URL: https://github.com/apache/paimon/pull/9402#issuecomment-5534936489

   [P2] Apply schema compatibility to the explicit-file scanner overload too
   
   `DataEvolutionGlobalIndexScanner.create(FileStoreTable, ..., Collection<IndexFileMeta>)` still feeds caller-supplied files directly into readers and coverage, while only the manifest-discovering overload filters incompatible entries. A caller that scans the manifest and passes those metas after an indexed type evolution can therefore open the old serializer and count the old range as covered, reintroducing the false-negative path this PR is fixing. Current vector planning usually passes already-filtered files, but direct callers of this public API are unprotected.
   
   Java metas now transport `schemaId`, so the overload can validate them against the pinned/current table schema before constructing coverage. Python has the same bypass in `data_evolution_global_index_scanner.py:166-181`, but its `IndexFileMeta` does not preserve schema identity, so that API needs either manifest entries or schema identity in the transported meta. Please add direct-overload regression tests after a type evolution.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "QuakeWang (via GitHub)" <gi...@apache.org>.
QuakeWang commented on PR #9402:
URL: https://github.com/apache/paimon/pull/9402#issuecomment-5425835436

   Agree. I will persist the nullable schema ID on IndexManifestEntry and perform compatibility filtering on manifest entries.
   
   To preserve the build-time identity across long-running Flink/Spark index builds, I plan to carry schemaId in IndexFileMeta only as committable transport, copy it to the top-level IndexManifestEntry field in ManifestEntryChanges, and restore it to the transport when an entry is read for delete, copy, or recommit flows. The index manifest will persist it only on IndexManifestEntry, not inside GlobalIndexMeta. Legacy entries will remain null and fail closed.
   
   Does this match your intent?


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9402:
URL: https://github.com/apache/paimon/pull/9402#issuecomment-5467324488

   Global Index schema compatibility issue: The `IndexFileMeta.schemaId` created by Lumina remains null, causing it to be immediately excluded by the compatibility filter. The local `LuminaVectorGlobalIndexScanTest` is failing (2/2), and the GitHub `build` and `build_test` jobs are failing for the same reason. The propagation of the Lumina `schemaId` needs to be implemented.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9402:
URL: https://github.com/apache/paimon/pull/9402#issuecomment-5462519951

   > Agree. I will persist the nullable schema ID on IndexManifestEntry and perform compatibility filtering on manifest entries.
   > 
   > To preserve the build-time identity across long-running Flink/Spark index builds, I plan to carry schemaId in IndexFileMeta only as committable transport, copy it to the top-level IndexManifestEntry field in ManifestEntryChanges, and restore it to the transport when an entry is read for delete, copy, or recommit flows. The index manifest will persist it only on IndexManifestEntry, not inside GlobalIndexMeta. Legacy entries will remain null and fail closed.
   > 
   > Does this match your intent?
   
   I think it is OK.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9402:
URL: https://github.com/apache/paimon/pull/9402#issuecomment-5748015795

   Requirement fit: SUPPORTED. Implementation: CLEAN.
   
   Rechecked the latest follow-up against the two previous findings. Both Java Generic/Sorted planners now partition compatible and incompatible entries, exclude incompatible coverage, schedule the full affected ranges, and delete old entries in the replacement commit. The Python planner mirrors that behavior and its tests commit the plan, verify legacy/type-changed entries are removed, and confirm replacements carry the current schema ID. The explicit-file scanner overloads in both Java and Python now filter with transported `schemaId` before reader construction and coverage.
   
   The current CI matrix, including Python, Spark, full-text and Lumina jobs, is green. The change provides end-to-end value by preventing old serializers from producing false-negative search results while preserving a normal rebuild path after upgrade or indexed-type evolution. I found no remaining actionable regression in the reviewed follow-up.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [core] Validate global-index schema compatibility before reader and coverage [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9402:
URL: https://github.com/apache/paimon/pull/9402#issuecomment-5534880849

   [P1] Make the rebuild path replace legacy/incompatible indexes
   
   This change makes every pre-upgrade global-index entry (`schemaId == null`) fail closed on reads, but the build planners still count those same entries as coverage. `GlobalIndexBuilderUtils.currentIndexEntries` returns them unchanged (lines 139-169), and both Generic/Sorted incremental scans feed them into `unindexedRowRanges`; the Python path does the same in `pypaimon/globalindex/build_plan.py:61-88` and returns early from `create_global_index.py:170-178`. Therefore a fully indexed table upgraded to this version has no usable index, while rerunning the normal create/build command silently schedules no work. With an appended tail, only the tail is rebuilt and the old ranges remain raw.
   
   Filtering those entries out of coverage alone is insufficient: the manifest combiner rejects adding overlapping global-index ranges while the old entries remain. Please partition current entries into compatible/incompatible sets, compute coverage only from compatible entries, add each incompatible range to `rangesToBuild`, and emit those incompatible entries as deletes in the same commit before adding replacements. The Python planner/commit path needs the same treatment.
   
   Please add upgrade tests for both Generic and Sorted planners (and Python): start with a legacy null-schema entry covering the whole table, assert an incremental build schedules the full range plus deletion, commit successfully, and verify the replacement carries the current schema ID. Also cover a non-null entry whose indexed type changed.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org