Skip to content

[core] Validate global-index schema compatibility before reader and coverage - #9402

Open
QuakeWang wants to merge 5 commits into
apache:masterfrom
QuakeWang:global-index-compat
Open

[core] Validate global-index schema compatibility before reader and coverage#9402
QuakeWang wants to merge 5 commits into
apache:masterfrom
QuakeWang:global-index-compat

Conversation

@QuakeWang

Copy link
Copy Markdown
Member

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

…overage

Global indexes are serialized with the indexed field types from their build schema, while readers use the current table schema. Reusing an incompatible index can miss matches, and counting it in coverage can skip the required data scan.

Persist the build schema ID in global-index metadata and compare indexed field types before reader grouping and coverage. Fail closed for legacy metadata and preserve the field across serializers and row-id reassignment.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should introduce schemaId to IndexManifestEntry, just like DataFileMeta.

@QuakeWang

Copy link
Copy Markdown
Member Author

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?

@JingsongLi

Copy link
Copy Markdown
Contributor

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.

Persist schema identity on top-level IndexManifestEntry while keeping IndexFileMeta as committable transport. Restore it for delete, copy, and recommit flows, and fail closed for legacy entries.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
@JingsongLi

Copy link
Copy Markdown
Contributor

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.

@JingsongLi

Copy link
Copy Markdown
Contributor

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

sourceTable, indexManifestEntries));
for (IndexManifestEntry indexManifestEntry : indexManifestEntries) {
boolean globalIndex = indexManifestEntry.indexFile().globalIndexMeta() != null;
if (globalIndex && !compatibleGlobalIndexes.contains(indexManifestEntry)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@JingsongLi

Copy link
Copy Markdown
Contributor

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

# Conflicts:
#	paimon-spark/paimon-spark-common/src/test/java/org/apache/paimon/spark/copy/CopyFilesUtilTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants