Repository navigation
[fix](paimon) Fail scans of binary partitions with legacy partition names - #69571
morningman merged 3 commits into
Conversation
…ames ### What problem does this PR solve? Issue Number: None Related PR: apache#69567 Problem Summary: Paimon finds the files of a partition under the directory its partition values name. With partition.legacy-name = true, the default, Paimon names a partition after the string of each value, which for a non-NULL BINARY / VARBINARY value is the Java array object ([B@1b6d3586) rather than its bytes, so every resolution of the name differs. The paths the connector planned for such a partition never existed: Paimon's own read fails with FileNotFoundException, while the BE skips missing files of an external table (ignore_not_found_file_in_external_table, default true), so a Doris query silently returned only the rows of the NULL partition (SELECT * of a table with rows in X'4142', X'' and NULL returned 1 row, while COUNT(*), answered from metadata, returned 3). The same happens to files written under a legacy name after the table switches to 'partition.legacy-name' = 'false', and to files written under the new names after it switches back. Scan planning now fails for a split whose binary partition value is not NULL when the split is looked up under legacy partition names, or when one of its data files was written under a schema whose options use legacy names, naming the table, the partition and the option. A split of the NULL partition, a file at an external path (found by the path its metadata records) and a COUNT(*) answered from metadata still plan as before. ### Release note Fix: a query of a Paimon table with a binary partition column whose partitions use legacy partition names ('partition.legacy-name' = 'true', the default) fails with an error naming the option, instead of silently returning only the rows of the NULL partition. ### Check List (For Author) - Test: Unit Test, Manual test - PaimonScanPlanProviderTest: legacy names fail planning while the NULL partition and COUNT(*) do not; a file written under legacy names fails after the option changes, and a file written under the new names fails after it changes back; a file at an external path still plans - Manual: local file:// Paimon catalog with tables created by the Paimon API - Behavior changed: Yes. A query that reads a non-NULL binary partition of a Paimon table with legacy partition names fails instead of returning a partial result. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two verified P2 issues: historical schema options can incorrectly reject readable binary-partition files, and the external-data exemption leaves the targeted silent row loss unfixed when a required deletion vector has a local legacy binary path. Both findings include concrete supported write sequences. Metadata COUNT and NULL-partition exceptions remain appropriate.
Critical checkpoint conclusions:
- Goal and proof: ordinary unreadable legacy binary data paths are rejected, but valid dynamic-option writes can also be rejected and external data with a local deletion vector can still silently lose rows. Tests omit both cases.
- Scope and clarity: changes are localized to the provider and its unit tests; existing branch resolution and authentication helpers are reused.
- Concurrency and locks: new arrays, suppliers and caches are local to a scan call; no new threads, shared mutable state, locks or lock-order changes.
- Lifecycle and initialization: caches end with scan planning; no new closeable resource, ownership cycle or static initialization dependency.
- Configuration: no new Doris configuration or dynamic-setting contract.
- Compatibility: no SPI, wire protocol or storage format change; the newly rejected readable-file case is the behavior regression reported inline.
- Thrift/protobuf IDs: neither directory changes.
- Parallel paths: native, Rust and JNI DataSplit routes share the guard; COUNT bypass is appropriate. Fallback branch selection, read-optimized wrappers and deferred JNI system-table paths were traced; the external-data/local-DV combination is the separate incomplete fix reported inline.
- Special conditions: the NULL exception matches SDK naming; an external data path proves nothing about its deletion-vector path, and persisted schema options do not prove writer options.
- Test coverage: inspected tests cover Parquet/ORC partition values, legacy failures, both persisted naming transitions, metadata COUNT, NULL keys and external file existence. Dynamic writer options and partial DELETE after external-root removal with local DV indexes are missing. No builds, unit tests or end-to-end tests were run under the static-only contract.
- Test results: no regression result files changed; new unit-test expectations match their constructed cases by inspection.
- Observability: the new exception includes table, partition and relevant file context; no new hot-path logging or metric requirement identified.
- Transactions and persistence: no Doris transaction/EditLog changes. External schema history is read only. The findings concern writer-option provenance and an existing deletion-vector-only commit path omitted by the new guard.
- Data writes and crash atomicity: no write path, commit protocol or resource acquisition is modified.
- FE/BE variable propagation: no transmitted fields or new variables require propagation across send paths.
- Performance: binary positions are computed once per scan; historical naming options are cached per branch identity and schema ID. Added work is linear in inspected files, with no separate demonstrated performance regression.
- Other issues and focus: no existing inline threads or additional user focus points were supplied. All changed hunks and both initial risk areas were reviewed; no other substantiated finding or unresolved static coverage gap remains.
Validation is static source analysis of the pinned PR head and Paimon 2.1.0 dependency sources; no runtime reproduction is claimed.
Review converged after 3 round(s).
| ? null : legacyOrcSplitTable((FileStoreTable) schemaTable.get(), split); | ||
| boolean legacyLookup = CoreOptions.fromMap((splitTable == null ? table : splitTable).options()) | ||
| .legacyPartitionName(); | ||
| for (DataFileMeta file : split.dataFiles()) { |
There was a problem hiding this comment.
[P2] Validate deletion-vector paths before exempting external data
An absolute externalPath() only makes the data file independent of partition naming. Create a binary-partitioned append table with bucket=-1, deletion-vectors.enabled=true, index-file-in-data-file-dir=true, default legacy names and external data paths. Insert two rows in one file, unset data-file.external-paths and its strategy, then delete only one row through Spark. Paimon's persistDeletionVectors commits only index changes using the current local path factory: the data keeps its recorded external path, but the new DV has a relative name under an identity-based binary directory. This continue exempts the data without checking its DV. The native reader's missing-DV NOT_FOUND reaches FileScanner, which skips the whole data range under the default ignore_not_found_file_in_external_table=true, silently losing the surviving row. This leaves the PR's targeted failure unfixed for that supported layout. Include required deletion files in the naming validation and cover this external-data/local-DV transition.
There was a problem hiding this comment.
Confirmed, and fixed in 65a1682d334. With index-file-in-data-file-dir = true, a deletion vector records an absolute path only if an external data path is configured when it is written (DeletionFileWriter.result). Otherwise Paimon resolves it under the partition's directory each time a split is planned (IndexInDataFileDirPathFactory.toPath). Under legacy names that directory is the identity string of a fresh array. So once the external paths are removed, a deletion vector written by a DELETE is never found, while its data file keeps its absolute path. Both native readers then get NOT_FOUND when they open the deletion vector (V1 _init_deletion_vector, V2 TableReader::_parse_delete_predicates), and FileScanner / FileScannerV2 skip the whole data range, surviving rows included. Paimon's own reader, JNI and Rust fail instead.
For a split with a non-NULL binary partition value on a table that keeps index files in data directories, every deletion vector the split carries must now exist when the split is planned, whether or not its data file has an external path. A missing one fails planning, naming the deletion vector file, its data file, the partition and the option. The option is immutable, so the reading table's value is the one every writer used. Deletion vectors under the table's index/ directory do not depend on partition names and are not checked. The existence check runs inside the catalog authentication, once per deletion vector file and scan. PaimonScanPlanProviderTest#externalBinaryPartitionFileWithLocalDeletionVectorFailsScanPlanning replays your sequence through the Paimon API: two rows written to one file with external paths, the paths removed, then one row deleted by persisting a deletion vector the way PaimonSparkWriter.persistDeletionVectors does it. Without the new check that split plans. #externalBinaryPartitionDeletionVectorAtExternalPathStaysReadable keeps the external paths, so the deletion vector records its absolute path and the split still plans.
master before this PR and branch-4.2 drop these rows too, since neither checks binary partitions at all. A BE-side change that makes a missing Paimon deletion vector an error instead of a skipped range, which would also cover deletion vectors lost in other ways, is tracked separately.
| } | ||
| if (legacyLookup || (splitTable != null && legacyPartitionNameSchemas | ||
| .computeIfAbsent(splitTable, ignored -> new HashMap<>()) | ||
| .computeIfAbsent(file.schemaId(), id -> withSchemaHistoryAuthentication(() -> CoreOptions |
There was a problem hiding this comment.
[P2] Do not infer writer naming options from the file schema ID
Paimon's FileStoreTable.copy(...) can override partition.legacy-name without persisting a new schema; TableSchema.copy retains the ID, which the writer stores in DataFileMeta. For example, create a binary-partitioned append table with the default true, write via table.copy(Collections.singletonMap("partition.legacy-name", "false")), then ALTER the persisted option to false. Those files have valid bytes-based paths and are readable with the current options, but still reference the original schema whose persisted option is true, so this branch now rejects every scan that reads them. The inverse dynamic override also shows that this schema boolean does not prove how a file was named. Check actual path/write provenance before declaring historical files unreadable, and add a test that writes with a dynamic naming option; the added ALTER-only tests do not cover it.
There was a problem hiding this comment.
Confirmed. FileStoreTable.copy keeps the schema ID (TableSchema.copy), so a file written with a dynamic partition.legacy-name = false (SET spark.paimon.partition.legacy-name = false, a Flink hint, or table.copy) refers to a schema whose persisted option is still true. Nothing in the metadata records the directory a file was written to: DataFileMeta has only the file name and an optional external path, and dynamic options are not persisted. The schema's option is therefore only a hint. Failing on it alone rejected files that are readable once the table is altered to false, and master reads them.
Fixed in 65a1682d334. The file's schema still selects which files to look at, but planning now fails only if such a file is also missing at its planned path (bucketPath/fileName, the path the native reader is given). In a table whose option was switched after legacy writes, the file is missing and the error stays. In your sequence the file is there and the scan plans. This costs one existence call per flagged file, and none for tables whose files were written under the current naming. PaimonScanPlanProviderTest#binaryPartitionFilesWrittenWithDynamicNamesStayReadable writes through table.copy({"partition.legacy-name": "false"}) on a table that keeps the default: planning fails as a legacy lookup until the option is altered to false, and then plans the file under the bytes of its value. Without the existence check it fails with the error you describe.
The inverse case is not covered: a file written with a dynamic partition.legacy-name = true while the schema says false is still looked up under its bytes, and the BE skips it. Catching that would take an existence call for every file of every binary partition on every scan. master and branch-4.2 behave the same.
TPC-H: Total hot run time: 28329 ms |
TPC-DS: Total hot run time: 150321 ms |
ClickBench: Total hot run time: 24.31 s |
FE UT Coverage ReportIncrement line coverage `` 🎉 |
FE Regression Coverage ReportIncrement line coverage |
…ary partitions ### What problem does this PR solve? Issue Number: None Related PR: apache#69571 Problem Summary: Follow-up to the scan-planning check of binary partitions in this PR, for two cases found in review. With 'index-file-in-data-file-dir' = 'true', a deletion vector records an absolute path only if an external data path is configured when it is written; otherwise Paimon looks it up under the directory of its partition when the split is planned, which under legacy partition names is named after a fresh Java array object. The check exempted every data file with an external path without looking at its deletion vector, so once the external data paths were removed from such a table, a deletion vector written by a DELETE was never found while its data file kept its absolute path: the native readers get NOT_FOUND when they open the deletion vector and the BE skips the whole data file, surviving rows included. For a split with a non-NULL binary partition value on such a table, every deletion vector the split carries must now exist, whether or not its data file has an external path; a missing one fails planning, naming the deletion vector file, the data file, the partition and the option. Deletion vectors under the table's index/ directory do not depend on partition names and are not checked. A writer's dynamic options (spark.paimon.*, Flink hints, Table.copy) keep the schema ID, so a file written with a dynamic 'partition.legacy-name' = 'false' on a table that keeps the default refers to a schema with legacy names while it sits under the bytes of its value. The check rejected such files after the table was altered to 'false', although Paimon and master read them. A file whose schema uses legacy names now fails planning only if it is also missing at its planned path (bucket path / file name). Both existence checks run inside the catalog authentication and are cached per path for the scan. A table that keeps its index files under index/ and whose files were written under its current naming makes no extra filesystem calls. ### Release note None (follow-up inside the PR) ### Check List (For Author) - Test: Unit Test - PaimonScanPlanProviderTest: a data file at an external path whose deletion vector was written after the external paths were removed fails planning; a deletion vector written while the external paths are configured still plans; a file written through table.copy({"partition.legacy-name": "false"}) on a table that keeps the default fails as a legacy lookup, and plans after the option is altered to false - fe-connector-paimon module tests: 851 run, 0 failures, 0 errors, 1 skipped (pre-existing) - Behavior changed: Yes. A scan of a non-NULL binary partition fails when a deletion vector it carries is missing, and a file whose schema uses legacy names fails it only when the file is missing at its planned path. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Addressed the first review round in
Three new tests in |
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed the complete supplied aggregate diff at 65a1682d33409db5bcb2146bfbb4ac7ac66dbccf: both changed files, their calling/reader paths, and the existing review context. One new verified P2 is inline: the binary guard rejects readable fallback partitions of an audit-log system table by consulting the main branch's naming option.
Existing findings were independently revalidated: 4366822772 is fixed because required deletion vectors are checked even for external data; 4366822776 is fixed because a historical schema hint now requires an actually missing reader path before rejecting a file. Neither is reposted. No existing P0/P1 was present. No additional user focus was supplied.
Flow checked: resolved/pinned table -> SDK pruning and split planning -> metadata COUNT(*) bypass -> binary guard -> native / JNI / Rust routing -> data and deletion-vector read paths.
| Required checkpoint | Conclusion |
|---|---|
| Goal and evidence | The targeted legacy-name layouts now fail during planning; the inline finding identifies a readable-layout regression. Six added SDK unit fixtures cover the intended fixes statically. |
| Scope and reuse | Changes stay in Paimon planning/tests and reuse authentication and branch helpers; the native schema-dictionary helper is too narrow for selecting system-table options. |
| Concurrency and locks | New maps/supplier belong to one synchronous plan invocation. No new worker, shared cache, lock order, or deadlock mechanism. Existing statement-plan reuse remains unchanged. |
| Lifecycle and initialization | Plan-local heap state expires with planning; no static initialization or native-resource ownership added. |
| Configuration | No Doris setting added. Existing naming and index-layout options are read from table generations; branch selection needs the inline correction. |
| Compatibility and rolling upgrades | No public SPI, function symbol, storage format, or serialized protocol changes. Rejection of unreadable layouts is intentional. |
| Thrift/protobuf IDs | No gensrc changes; branch-ID comparison is inapplicable. |
| Parallel paths | Checked native Parquet/ORC, JNI, Rust, count pushdown, external files, NULL keys, ordinary fallback and system wrappers. Normal data/$ro branch selection is preserved; audit-log fallback is the finding. Non-DataSplit chain/format paths remain JNI. |
| Special conditions | Non-NULL detection, count exemption, external data/DV independence, writer-schema hints and actual paths have concrete rationale. The guard must also use the producing branch for wrappers without a native dictionary. |
| Test coverage | All six new tests, their real committed-file/DV helpers and the adjusted binary Parquet/ORC test were inspected. Missing wrapper-plus-fallback binary coverage corresponds to the inline defect. |
| Expected results | Added assertions match the intended planned paths/errors and metadata counts on inspection. No result files changed. Tests were not executed. |
| Observability and failures | Errors identify table, binary partition, relevant file and option; IO errors retain path/cause. Schema/exists IO uses the authenticated boundary. No swallowed failure or separate critical metric omission found. |
| Transactions, persistence and failover | Production is read-only; SDK snapshot selection, EditLog and commit/failover protocols are unchanged. |
| Writes, atomicity and crash cleanup | No production write or transaction state added. Errors discard planning results; no new crash-leak mechanism found. |
| FE/BE variables | None added; checked data/DV paths are the existing paths passed to readers. |
| Performance and memory | CPU/heap grow with selected metadata; schema facts memoize per branch/schema and existence checks per path per scan. Schema IO is once per branch/schema; existence IO is limited to flagged historical data and relevant DVs. No additional demonstrated redundant operation. |
| Other core invariants | No change to committed visibility, version limits, DV offsets/cardinality or attachment to native subranges. BE allocator/MoW bitmap concerns are inapplicable to this Java planning change. No other verified new issue. |
Validation limits: static review only, as required; no builds, tests, clusters, runtime reproductions or source edits. The new tests do not execute a BE/remote-storage reader matrix or inject authentication/IO failures. The already-discussed inverse dynamic override (writer legacy=true while its persisted schema says false), unrelated missing-file layouts, and files disappearing after planning remain pre-existing limits of this targeted guard and are not reported again.
Review converged in three rounds: both complete-scope reviewers and the separate path-risk reviewer returned NO_NEW_VALUABLE_FINDINGS on the unchanged final payload. All risk focuses and candidates have an explicit disposition; no unresolved review candidate remains. The verified P2 still needs correction; convergence describes review completion, not a bug-free verdict.
Review converged after 3 round(s).
| int binaryField = nonNullField.getAsInt(); | ||
| FileStoreTable splitTable = schemaTable.get() == null | ||
| ? null : legacyOrcSplitTable((FileStoreTable) schemaTable.get(), split); | ||
| Table optionsTable = splitTable == null ? table : splitTable; |
There was a problem hiding this comment.
[P2] Resolve fallback options for audit-log binary partitions
For $audit_log, resolveSchemaDictTable returns null, so this selects the system wrapper's options. Paimon's audit wrapper exposes the main branch's options while still planning and reading the fallback branch. For example, write only a NULL binary partition on main with the default partition.legacy-name=true; create a backup branch, set its persisted naming option to false, and write a non-NULL binary partition there. After setting main's scan.fallback-branch=backup, SELECT id FROM t$audit_log now throws the legacy-name error for the backup split, even though its byte-named file exists and the JNI reader selects backup correctly. Resolve the pinned system source and the split's producing branch independently of native-schema-dictionary availability, and add this wrapper/fallback case to the tests. This uses ordinary persisted branch options, with no dynamic writer override or schema-ID provenance ambiguity.
There was a problem hiding this comment.
Confirmed, and fixed in cf79a1ac228. resolveSchemaDictTable has no answer for $audit_log, since an AuditLogTable is a DataTable rather than a FileStoreTable. The check then fell back to the wrapper's own options, and AuditLogTable.options() returns those of the fallback pair, which are the main branch's. The splits still come from FallbackReadScan, which plans a partition that is missing on main from the fallback branch and reads it with that branch's read. I replayed your history with the Paimon 2.1.0 API. Main keeps the default partition.legacy-name = true and holds only a NULL partition, and backup has false and a row in key = 0x4142. $audit_log reports true, while the fallback split's file is at key=AB/bucket-0/…, exists, and Paimon's own read returns the row. The check rejected that scan although master reads it through JNI.
For the system tables read through JNI, the check now resolves the table that produced the splits from the wrapper's pinned source, the same source the timestamp-history checks already use (resolveTimestampHistoryTable). It then picks each split's branch as it does for data tables, so a fallback split of $audit_log or $row_tracking is judged by the naming and schemas of its own branch. In the reverse history (main false, backup true) the scan now fails at planning with the legacy-name error. Before, it planned and Paimon's reader failed at execution with a FileNotFoundException. $binlog is not affected: it exposes its partition columns as arrays, so it has no binary partition column to check. fallbackBinaryPartitionsUseTheNamingOfTheirBranch covers both directions for the data table and $audit_log, and reads the planned $audit_log splits back through the wrapper.
While there: a scan.file-creation-time-millis read plans a fallback table through the pair's SnapshotReader, which returns plain DataSplits of the wrapped branch. legacyOrcSplitTable cast every split to FallbackSplit, so such a scan of a table with a non-NULL binary partition failed with a ClassCastException. A split that is not a FallbackSplit now maps to the wrapped branch. The same cast on the native timestamp path predates this PR and is fixed with it. fileCreationTimeScanOfFallbackTableChecksBinaryPartitionsOfTheMainBranch plans such a read.
TPC-H: Total hot run time: 28816 ms |
TPC-DS: Total hot run time: 151019 ms |
ClickBench: Total hot run time: 24.52 s |
FE UT Coverage ReportIncrement line coverage `` 🎉 |
…tions of their branch ### What problem does this PR solve? Issue Number: None Related PR: apache#69571 Problem Summary: Follow-up to the scan-planning check of binary partitions in this PR, for two cases found in review. The check takes the naming option and the file schemas from the branch that produced each split, but only when the table has a schema dictionary. For $audit_log and $row_tracking, which are read through JNI, there is none, so the check used the wrapper's own options, and an AuditLogTable reports the options of the fallback pair, which are the main branch's. With scan.fallback-branch set, a partition missing on the main branch is still planned and read under the fallback branch's options. A table whose main branch keeps the default 'partition.legacy-name' = 'true' and holds only a NULL partition, while its fallback branch uses 'false' and holds a non-NULL binary partition, was therefore rejected for $audit_log although Paimon reads the fallback rows; in the reverse layout the scan planned and Paimon's reader failed with FileNotFoundException at execution. For the wrappers read through JNI the check now resolves the table that produced the splits from the wrapper's pinned source, as the timestamp-history checks already do, and picks each split's branch from it as it does for data tables. $binlog exposes its partition columns as arrays and is not checked. A scan.file-creation-time-millis read plans a fallback table through the pair's SnapshotReader, which reads the wrapped branch and returns plain DataSplits. Picking the branch cast every split to FallbackSplit, so such a scan of a table with a non-NULL binary partition failed with a ClassCastException (the same cast on the native timestamp path predates this PR). A split that is not a FallbackSplit now maps to the wrapped branch. ### Release note None (follow-up inside the PR) ### Check List (For Author) - Test: Unit Test - PaimonScanPlanProviderTest: a fallback pair whose main branch holds only a NULL partition and whose fallback branch holds a non-NULL binary partition plans the data table and $audit_log, and the $audit_log splits read back both rows through Paimon, when the fallback branch names partitions by bytes; both fail at planning with the legacy-name error when only the main branch does; a scan.file-creation-time-millis read of a fallback pair with a non-NULL binary partition plans the main branch's file - fe-connector-paimon module tests: 853 run, 0 failures, 0 errors, 1 skipped (pre-existing) - Behavior changed: Yes. $audit_log and $row_tracking of a fallback table check binary partitions under the options of the branch that produced each split, and a file-creation-time read of a fallback table with a binary partition no longer fails with a ClassCastException. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Addressed the second review round in
Two new tests in |
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed the complete authoritative diff at cf79a1a. No new substantiated finding. Two complete-review agents and a separate branch-provenance risk reviewer returned NO_NEW_VALUABLE_FINDINGS; the main review independently checked their evidence and reconciled every risk.
The reported triggers in existing P2 threads 4366822772 (external data/local deletion vector), 4366822776 (readable dynamic byte-named files), and 4367401232 (audit-log fallback options) are addressed at this head. No duplicate inline comments are proposed, and no existing P0/P1 finding was confirmed. The inverse dynamic-option limitation discussed in 4366822776 remains explicitly outside the fix.
Individual critical-checkpoint conclusions:
- Goal and correctness: the guard rejects known unreadable non-NULL binary partition files during planning, before native missing-file skipping can hide rows. NULL-only partitions, readable external/dynamic byte-named files and metadata-only COUNT retain their intended paths. The added real-file unit fixtures support these conclusions by inspection; they were not executed here.
- Scope and reuse: changes are confined to the scan provider and its tests. Existing branch resolution, pinned-source handling and authenticated execution are reused; no unrelated change was identified.
- Concurrency and locking: new maps and memoized suppliers are local to one planning invocation. No new shared mutable state, thread, lock, lock-order edge or heavy operation under a new lock is introduced.
- Lifecycle and initialization: the new caches end with scan planning; no production closeable resource, ownership cycle, worker lifecycle or static initialization dependency is added.
- Configuration: no Doris configuration item is added. Checks use effective scan/branch options; the existing deletion-index layout option is immutable, while naming-option transitions are handled explicitly.
- Compatibility: no public SPI, function symbol, storage format or serialized scan representation changes. Earlier failure for unreadable physical layouts is the intended behavior change.
- Thrift/protobuf IDs: no gensrc definitions change, so the cross-release field/enum-ID audit is inapplicable.
- Parallel paths: validation precedes native/JNI/Rust data-reader selection and follows the file-less merged-count short-circuit. Audit-log, row-tracking and read-optimized sources retain the relevant branch provenance. Reversed fallback order matches SDK dispatch; plain SnapshotReader splits originate from the wrapped branch. Nested chain results use non-DataSplit JNI dispatch, and ordinary data-chain reads are already rejected upstream.
- Conditional checks: NULL and external-data exemptions follow actual path semantics. Historical schema naming is only a hint and is combined with absence at the planned path; external data still receives required deletion-vector validation. Comments explain these distinctions, and no new speculative defensive branch was identified.
- Test coverage: eight added tests exercise legacy lookup, NULL/count, both persisted naming transitions, dynamic byte naming, external data, local/external deletion vectors, fallback/audit behavior and file-creation-time scans. The existing binary ORC/Parquet fixture now uses byte naming. Dedicated binary fixtures are not added for every wrapper, reversed priority, BINARY(n), multiple binary keys, explicit Rust or session-forced JNI; those routes were checked statically. No new end-to-end suite is included.
- Test results: no regression .out files change. The expected errors, paths, counts and sorted audit rows are consistent with the inspected setup and SDK semantics. Builds, tests, checkstyle and cluster behavior were not run or independently verified in this static-only invocation.
- Observability and errors: failures include database/table, binary partition, relevant file and naming/layout option; filesystem exceptions preserve path and cause. No new background operation requiring metrics or additional INFO logging is introduced.
- Transactions and persistence: production changes only read scan metadata/files. No EditLog, commit, persistence, visibility-version or master-failover protocol changes.
- Data writes and atomicity: no production write path changes; added SDK writes/commits are test fixtures. The guard does not change baseline races with files removed after planning.
- FE-BE variables: no new transmitted variable or sending path; both native scanner implementations and existing JNI/Rust payload forms consume the same range shapes.
- Performance and memory: binary-field discovery is once per scan; schema naming is cached by table identity/schema ID and existence by path. Work is linear in relevant files, not native subranges; metadata count and NULL-only splits avoid new file I/O. No new shared unbounded cache or native memory ownership is introduced.
- Other issues and focus: the final missed-issue rechecks found no additional substantiated regression. No additional user focus was supplied. All changed hunks and initial risks have evidence-backed dispositions.
Remaining limits: this is a static review, with no runtime or author-test-result verification. As already disclosed in the PR and existing thread, a dynamic legacy-name=true writer (or equivalent old writer) under a persisted false schema can still evade the historical-schema hint. Binlog exposes ARRAY partition fields and retains existing SDK/JNI errors rather than this scalar binary guard. These known boundaries are not new inline findings. There is no unresolved candidate or unreviewed changed code within the agreed static review scope.
Review converged after 1 round(s).
TPC-H: Total hot run time: 28143 ms |
TPC-DS: Total hot run time: 149955 ms |
ClickBench: Total hot run time: 24.16 s |
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: #69571, #68142 Problem Summary: **In short.** When a Paimon split's deletion vector file is missing, the native readers report `NOT_FOUND`, and both file scanners take that for a missing data file and skip the whole range. The query succeeds and silently drops every surviving row of the data file. This PR reports a missing deletion vector as an error, so the query fails instead. A missing data file is still skipped as before. **Background.** A Paimon split for a table with deletion vectors names a data file and the deletion vector that marks its deleted rows. The native readers open the deletion vector before they read a row: V1 in `PaimonParquetReader` / `PaimonOrcReader::_init_deletion_vector`, V2 in `TableReader::_parse_delete_predicates`, both through `DeletionVectorReader`. `FileScanner` and `FileScannerV2` skip a range whose reader reports `NOT_FOUND` while `ignore_not_found_file_in_external_table` is on (the default). That setting exists for file listings that are cached by the FE and may name a file that has since been removed. #68142 already exempted Iceberg ranges, whose files all belong to a committed snapshot. **The problem, and what it cost.** A deletion vector is never a stale listing: it belongs to the snapshot that planned its data file. It can still be missing, for example when Paimon expires the snapshot that wrote it while the data file stays live under a newer one, or when it is looked up under a partition name that is never found (#69571). The data file is still there, but the scanners skip the whole range, so its surviving rows are dropped together with the deleted ones. The BE logs `Skip missing file` with the path of the data file, which exists. Paimon's own reader, the JNI reader and the Rust reader fail on the same split instead. **How this PR fixes it.** `DeletionVectorReader`, which opens every deletion vector for V1 Paimon, V1 Iceberg and the V2 table reader, now reports a missing file as `INTERNAL_ERROR` with the path of the deletion vector, both when it opens the file and when it reads it. The scanners only skip `NOT_FOUND`, so the query fails. A missing data file still reports `NOT_FOUND` and is skipped as before, with or without a deletion vector. For Iceberg, which already failed on any missing file (#68142), only the error code of a missing deletion vector changes. One boundary: if the data file is gone as well, the result depends on which file the reader reaches first. FileScannerV2 parses the deletion vector while it prepares the split, before it opens the data file, so such a split now fails; in the local run below V1 Parquet failed too, while V1 ORC, which opens the data file first, still skipped it. **Results.** A local cluster with a `file://` Paimon catalog, two append tables (Parquet and ORC, `deletion-vectors.enabled = true`), five rows each, the first row deleted through a deletion vector the way a Spark `DELETE` persists it: | Files on storage | Reader | Before | After | |---|---|---|---| | data file and deletion vector | V1 / V2, Parquet / ORC | `2,3,4,5` | `2,3,4,5` | | data file only (deletion vector removed) | V1 / V2, Parquet / ORC | empty result, query succeeds; `Skip missing file: path=<data file>` in be.INFO | `[INTERNAL_ERROR]Deletion vector file is missing: path .../index/index-...-0, failed to get file size ...: No such file or directory` | | deletion vector only (data file removed) | V1 / V2, Parquet / ORC | empty result | empty result (unchanged) | | neither | V2, V1 Parquet | empty result | the deletion vector error above | | neither | V1 ORC | empty result | empty result (unchanged) |
What problem does this PR solve?
Issue Number: None
Related PR: #69567
Problem Summary:
In short. A query of a Paimon table with a BINARY / VARBINARY partition column now fails with an error naming
'partition.legacy-name'when it would read a non-NULL binary partition whose files Paimon cannot locate, instead of silently returning only the rows of the other partitions. This is the read-side counterpart of #69567, which rejects writes to such tables.Background. Paimon looks for the files of a partition under the directory its partition values name, computed when the split is planned; a writer puts the files under the name it computes when it writes. With
partition.legacy-name = true, the default, the name of a value is itstoString(), which for a binary value is the Java array object ([B@1b6d3586) rather than its bytes, so every resolution differs. Withpartition.legacy-name = falseit is the text of the bytes (p=ABforX'4142'). The option can be changed on an existing table, which changes how existing partitions are looked up but does not move their files. The BE skips a file of an external table that no longer exists (ignore_not_found_file_in_external_table, default true), which is meant for files removed between planning and reading.The problem, and what it cost. Measured on master with Paimon 2.1.0, on a
file://Paimon catalog of a local FE + BE, with tables created and written through the Paimon API:X'4142',X'EF', NULLFileNotFoundException(p=%5BB@...)SELECT *returns only the NULL row;COUNT(*), answered from metadata, returns 3X'4142'was written, then'partition.legacy-name' = 'false'and rows inX'4344', NULLp=AB, the file is underp=%5BB@...X'4142'row is missingX'4142'and NULL were written, then'partition.legacy-name' = 'true'So the query succeeds with a partial result and nothing tells the user. Following the obvious fix, setting
'partition.legacy-name' = 'false', does not make the old partitions readable either; it only moves the loss to the files written before the change.How this PR fixes it.
PaimonScanPlanProvider.planScanInternalchecks every data split that will read files (after the splits whoseCOUNT(*)is answered from metadata) whose binary partition value is not NULL:schemaId, looked up once per schema and branch, inside the catalog authentication) and is also missing at its planned path (bucketPath/fileName, the path the native reader is given), planning fails and names that file. The schema alone does not prove how a file was named: a writer's dynamic options (spark.paimon.*, Flink hints,Table.copy) keep the schema ID, so a file written with a dynamic'partition.legacy-name' = 'false'on a table that keeps the default refers to a schema with legacy names while it sits under the bytes of its value, and it is read once the table uses those names;'index-file-in-data-file-dir' = 'true', every deletion vector the split carries must exist, whether or not its data file has an external path, or planning fails and names the deletion vector file and its data file. Such a table keeps its deletion vectors with its data files: one written while no external data path is configured records only its file name and is looked up under the directory of its partition, which a legacy name never finds, while its data file keeps its external path, and the BE skips the whole data file when it cannot open the deletion vector. The option is immutable, so every writer used the reading table's value. Deletion vectors under the table'sindex/directory do not depend on partition names and are not checked.A NULL partition is named after
partition.default-nameunder both namings, and a data file with an external path is read from the path its metadata records, so neither is rejected; the deletion vectors of a file with an external path are still checked. The system tables read through JNI ($audit_log,$row_tracking) report the options of the main branch, while a fallback split is planned and read under its own branch, so the check resolves the branch that produced each split from the pinned source of the wrapper, as it does for data tables.$binlogexposes its partition columns as arrays and is not checked; Paimon's own reader fails on a file it cannot find. Ascan.file-creation-time-millisread of a fallback table plans the main branch through itsSnapshotReader, and its plain splits are judged under the main branch. The error names the table, the partition and the option. The existence checks run inside the catalog authentication, once per path and scan: a table that keeps its index files underindex/and whose files were written under its current naming makes no extra filesystem calls.Not covered. A file written with a dynamic
'partition.legacy-name' = 'true'(or by a Paimon writer too old to know the option) while its schema says'false'sits under a legacy name but is looked up under its bytes, and the BE still skips it, as on master and branch-4.2. Catching it would take an existence call for every file of every non-NULL binary partition on every scan.Results. On the same local cluster, after this PR:
SELECT *of the legacy table (also withforce_jni_scanner = true)Cannot read Paimon table d.api_legacy: the table uses legacy partition names ('partition.legacy-name' = 'true'), under which Paimon names partition p = 0xEF after a Java array object rather than its bytes, so the files of the partition are never found. Paimon cannot read the partition either; ...SELECT COUNT(*)of the legacy tableSELECT * ... WHERE p IS NULLof the legacy tableSELECT *of the table switched to new names... data file data-b3abc825-...-0.parquet of partition p = 0x4142 was written while the table used legacy partition names ..., the file the Paimon API read reports as not foundSELECT *of the table switched back to legacy names... the table uses legacy partition names ...SELECT *of the legacy table with only NULL partitions, and of tables with new namesPaimonScanPlanProviderTestadds eight tests: a scan under legacy names fails while the NULL partition andCOUNT(*)still plan; a file written under legacy names fails after the option changes to false, and a file written under the new names fails after it changes back to true; a file at an external path still plans and points at an existing file; a file at an external path whose deletion vector was written after the external paths were removed (persisted the wayPaimonSparkWriter.persistDeletionVectorsdoes it for a DELETE) fails and names the deletion vector, while a deletion vector written with the external paths in place still plans and points at an existing file; a file written throughtable.copy({"partition.legacy-name": "false"})on a table that keeps the default fails as a legacy lookup, then plans once the option is altered tofalse; a fallback table whose main branch holds only a NULL partition and whose fallback branch holds a non-NULL binary partition plans the data table and$audit_logwhen the fallback branch names partitions by bytes, and the planned$audit_logsplits read both rows back through Paimon, while both fail at planning with the legacy-name error when only the main branch names them by bytes; and ascan.file-creation-time-millisread of a fallback table with a non-NULL binary partition plans the main branch's file.nativeBinaryPartitionsKeepAllPathValues, added by #68786 for the rendering of binary partition values, now creates its table with'partition.legacy-name' = 'false': under legacy names the files it planned did not exist. The new tests fail when the lookup check, the check of the files' schemas, the exemption of external paths or of NULL values is removed, when the check moves before theCOUNT(*)splits, when the deletion vectors are not checked, when every deletion vector fails without its existence being checked, when a file whose schema uses legacy names fails without its existence being checked, when a system table read through JNI is checked under the wrapper's options, when every split is judged under the main branch, or when a split that does not come from the fallback scan is cast to a fallback split (the file-creation-time read then failed with aClassCastException). The 853 tests of fe-connector-paimon run without failures (one is skipped, as before).No regression test is added: Doris cannot create a binary column, so the tables would have to come from the Spark container of the Paimon docker environment, which was not run locally.
Release note
Fix: a query of a Paimon table with a binary partition column fails with an error naming 'partition.legacy-name' when it would read a non-NULL binary partition whose files Paimon cannot locate under legacy partition names, instead of silently returning only the rows of the other partitions.
Check List (For Author)
Test
Local FE + BE with
CREATE CATALOG ... PROPERTIES ('type' = 'paimon', 'warehouse' = 'file:///...'); tables created and written with the Paimon Catalog API (Schema.newBuilder().column("p", DataTypes.BYTES()).partitionKeys("p"), the option changed withSchemaChange.setOption); the queries above, compared with a read through the Paimon API. The cluster results above were measured before the follow-up commits for the deletion vectors, the dynamic naming option and fallback branches, which are covered by unit tests only; none of the tables above has a deletion vector, a fallback branch or a file written with a dynamic option.Behavior changed:
A query that reads a non-NULL binary partition of a Paimon table whose files are named or looked up under legacy partition names fails instead of returning a partial result.
Does this need documentation?
Check List (For Reviewer who merge this PR)