Repository navigation
[fix](paimon) Preserve schema options across renamed-field reads - #69545
Merged
Gabriel39 merged 2 commits intoOct 25, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Two P2 regressions remain in historical option restoration: obsolete startup options can make a valid snapshot read fail, and inconsistent fallback priority can make the selected rows depend on FE CPU count. Both inline findings include concrete triggers and fix directions. No existing inline threads or blocking comment IDs were supplied, and there was no additional user focus.
All nine changed files and their binding, planning, metadata, and backend interactions were reviewed statically. Checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and data correctness | Historical fields and sequence/aggregation options are restored together, and current schema-only renames survive JNI reconstruction. The inline findings identify uncovered startup-state and fallback-priority regressions. |
| Scope, clarity, and reuse | Focused Paimon changes; common restoration and SDK fallback translation are reused. |
| Concurrency and locks | Local maps and table copies; no new threads, locks, shared mutable state, or lock order. Existing statement caches and retained-schema checks remain in use. |
| Lifecycle and static initialization | Bound schemas survive catalog-loader removal; table and secondary system-source payloads were traced. No new owned resource or static-initialization dependency. |
| Configuration | No new setting; existing relation/catalog/physical reader-option precedence is retained through provenance markers. |
| Compatibility | No SPI, function-symbol, storage-format, serialized-class-shape, or protocol-definition change. FE and BE use the same pinned SDK version. |
| Thrift and protobuf IDs | Not applicable; no definitions changed. |
| Parallel paths | Examined metadata/schema/partition/statistics, current and historical selection, OPTIONS/incremental resets, native/JNI, system wrappers, and independent fallback branches. |
| Conditional checks and errors | Changed type/provenance checks correspond to SDK behavior; empty-timestamp fallback preserves SDK error handling. Historical scan-state restoration causes the reported validation failure; restored branch-policy options can disagree with the retained fallback dispatch flag. |
| Test coverage | Inspected all 18 new unit tests and 24 regression query checkpoints across both force_jni_scanner settings, plus related existing tests. Removed historical scan mode and primary-to-fallback policy evolution with overlapping partitions are missing cases. |
| Expected results | Added rows agree with writes and selectors, including sequence retention and aggregation totals 22/27; result ordering is deterministic. |
| Observability | Existing contextual metadata/scan errors are sufficient for this focused change; no new service or recurring operation needs metrics. |
| Transactions, persistence, and failover | No production transaction, EditLog, durability format, or failover protocol change. |
| Writes and atomicity | Production writes are unchanged; new work builds disposable read-side copies. |
| FE/BE value passing | No new wire variable; selected schemas/options remain in the existing serialized table and system-source payloads. |
| Performance | Additional bounded snapshot/schema reads occur at copy boundaries; no new per-row work or unbounded data enumeration was identified. |
| Other concerns and limits | No additional verified issue after independent rechecks. Static review only: no builds/tests were run, runtime reader routing was not proven, and generated-output provenance was not independently verified. |
A statistics authentication-boundary candidate was dismissed after tracing the existing authentication requirements for binding and scans; no separate otherwise-successful query regression was established.
Review converged after 3 round(s).
### What problem does this PR solve? Historical schema options could revive a removed startup mode or obsolete primary-branch policy. The former rejects valid snapshot reads; the latter changes selected rows when manifest parallelism normalization rebuilds a fallback reader. Preserve effective selectors and branch policy, including absent keys, while restoring historical field options. ### Release note Fix historical Paimon reads after removing startup options or changing primary/fallback branch priority. ### Check List (For Author) - Test: Full Paimon connector install and Checkstyle; real-table unit tests for removed scan mode and overlapping branches at capacities 64/128; cluster regression with both scanner settings. - Behavior changed: Yes. Historical schema restoration retains effective read state and branch policy. - Does this need documentation: No.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Static review of all 9 changed files at 61e929f: one new P2 finding. Historical field-option restoration also rewinds schema-independent reader settings, so a corrected physical batch size can become invalid again on historical reads.
Existing-thread dispositions:
- Removed startup mode, thread 4359737386: the reported latest-full resurrection is fixed; absent startup keys and aliases are preserved.
- Branch priority, thread 4359737377: the original primary-to-fallback example is fixed. The same already-reported priority-reconstruction concern still applies when historical chain-table.enabled=true is restored over a current false primary pair: manifest-cap reconstruction can infer a different priority at capacities 64 and 128. This is covered by that existing thread and is not reposted.
- No existing P0/P1 finding was present to carry forward. No additional user-provided focus was supplied.
Critical checkpoint conclusions:
- Goal: renamed sequence/aggregation field options are restored alongside the selected columns, and statement-bound current schemas survive JNI loader removal; the runtime-reader regression prevents full correctness.
- Scope and reuse: changes use shared copy and system-source boundaries; restoring the entire historical option map is broader than the field-dependent repair requires.
- Concurrency and locks: no new thread, lock, lock order, or shared mutable ownership is introduced; maps are copied and existing statement pins remain in place.
- Lifecycle and initialization: copied tables retain their FileIO/catalog context; catalog loaders are removed only for backend transport, then the exact schema is restored. No new static-initialization or close-ownership issue found.
- Configuration: no new setting; catalog provenance and relation overrides are retained, but corrected physical reader tuning is lost in the reported case.
- Compatibility: no public SPI, wire carrier, or persistent format change identified.
- Thrift/protobuf: no gensrc changes.
- Parallel paths: inspected binding, partition/statistics metadata, snapshot/timestamp/tag/OPTIONS, incremental resets, fallback/chain handling, system and non-file sources, and native/JNI execution. No additional distinct omission verified.
- Conditions and errors: file/system/statement/chain guards have concrete roles; SDK selector-error fallback remains SDK-compatible and later scan validation owns errors. The new reader-setting failure is reported inline.
- Tests: inspected 21 real-table unit tests and 26 ordered regression query checkpoints for both force_jni_scanner values. Coverage includes renamed fields, warm/current schemas, historical selectors, independent branches, explicit catalog precedence, serialization, and removed startup mode; it omits repaired physical reader settings and the chain-history priority variant.
- Expected results: sequence winners and aggregation sums match fixture writes; no incorrect output block found. Static inspection cannot verify that output files were generated by a runner.
- Observability: existing option-validation errors and metadata exception logging cover these transformations; no new metrics requirement identified.
- Persistence/failover: no EditLog or transaction-persistence change.
- Data writes/atomicity: production changes affect reads and copies, not commit/write protocols.
- FE-BE propagation: no new transport variable; both scanners use the selected schema, and JNI receives the loader-free equivalent.
- Performance: additional selector/schema metadata reads and option-map copies are per table/branch; no demonstrated data-size-dependent or lock-held regression.
- Further recheck: reviewed null resets, aliases, catalog override equality, physical-option removal, REST rebinding order, fallback reconstruction, system-source transport, and statistics authentication. No other distinct substantiated finding remains.
Limits: static inspection only; no build, test, cluster, or Kerberos execution. The read-optimized system-table test may use an empty SDK baseline and therefore proves schema/copy parity more strongly than nonempty row behavior. Review convergence describes completion of the bounded review, not absence of bugs.
Review converged after 3 round(s).
| .add(CoreOptions.BRANCH.key()) | ||
| .add(CoreOptions.SCAN_PRIMARY_BRANCH.key()) | ||
| .add(CoreOptions.SCAN_FALLBACK_BRANCH.key()) | ||
| .build(); |
Contributor
There was a problem hiding this comment.
[P2] Preserve current reader tuning when restoring historical fields
This new preserve set omits schema-independent reader options. Consider a Paimon table that commits snapshot 1 with read.batch-size=131072 (valid in the SDK), then changes that physical option to 1024. With no catalog or relation batch override, FOR VERSION AS OF 1 calls restoreBoundSchema, which starts with the old 131072; current 1024 equals the current persisted value, so the delta loop does not copy it, and this set does not preserve it either. Final PaimonReaderOptions.validateEffectiveTable therefore rejects the historical query against Doris's maximum of 65536, while the previous SDK copy kept 1024 and succeeded. This is a reader-tuning omission in the new set, separate from the startup-selector conflict in existing thread 4359737386. Retain effective current reader tuning, including removed keys, while restoring historical field-dependent options, and add a regression for a repaired physical setting.
Contributor
TPC-H: Total hot run time: 28184 ms |
Contributor
TPC-DS: Total hot run time: 149565 ms |
Contributor
ClickBench: Total hot run time: 24.16 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28247 ms |
Contributor
TPC-DS: Total hot run time: 150377 ms |
Contributor
ClickBench: Total hot run time: 24.19 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
FE Regression Coverage ReportIncrement line coverage |
2 similar comments
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
924060929
approved these changes
Oct 24, 2026
4 tasks done
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Oct 29, 2026
Apply the outstanding changes in master merge order: apache#69545 7d16729 apache#69551 7224a2d apache#69612 61d1aef apache#69615 8d2bd1e apache#69650 3dc7341 apache#69651 c00ecb1 apache#69646 e493c61 apache#69668 7865646 apache#69614 already exists on branch-4.2 through apache#69644. Adapt datasource APIs and tests to the release branch, retaining its existing scanner dispatch and statement-bound metadata contracts.
yiguolei
pushed a commit
that referenced
this pull request
Oct 30, 2026
### What problem does this PR solve? Backport the requested external-data and FileScannerV2 changes to `branch-4.2` in their actual master merge order: | Order | Master PR | Change | Backport | | --- | --- | --- | --- | | 1 | #69545 | Preserve Paimon schema options after field renames | Included | | 2 | #69551 | Expand Iceberg capability, transaction and snapshot tests | Included with 4.2 API adaptations | | 3 | #69614 | Elasticsearch HTTP scans in FileScannerV2 | Already present through #69644 (`2104b7509cda`); not applied twice | | 4 | #69612 | Check catalog visibility before SHOW DATABASES accesses metadata | Included | | 5 | #69615 | Publish Iceberg Parquet split offsets | Included | | 6 | #69650 | Preserve ES compound residuals and expand scanner coverage | Included | | 7 | #69651 | Accept Elasticsearch 9.x during node discovery | Included | | 8 | #69646 | Prioritize Parquet V2 row groups using TopN bounds | Included | | 9 | #69668 | Arrow loads and schema inference in FileScannerV2 | Included | The eight outstanding changes are combined in one commit. This branch retains the 4.2 FE datasource APIs instead of importing master's connector-module refactor. ### Backport adaptations - Port Paimon schema restoration, catalog-option provenance, fallback handling and tests to the existing table-copy/serialization lifecycle. Keep statement-bound fields when reader options are applied after a schema-only rename. - Exercise Iceberg's existing metadata and transaction entry points with real in-memory tables and retained-generation fixtures. Preserve the nine disabled known-bug reproductions; omit two connector-only lifecycle tests. Expiration checks verify live data-file sets and exact delete-file counters; 4.2's separate manifest-name/data-counter issue remains unchanged. The delete/history SQL suite checks visible rows and eager/late-materialization parity because 4.2 does not expose master's `_file`/`_pos` columns. The inventory labels upstream-only evidence explicitly. - Port ES residual tracking to the legacy planner and add compound OR/NOT, docvalue eligibility and Thrift-context tests there. Use Java 8-compatible HTTP APIs for opt-in ES 9 tests and the branch's supported node-address fixtures. - Keep the existing full-schema Iceberg metadata-table scan, which already supplies the file-size dependency for split offsets. Adapt the external write suite to verify offsets without requiring master-only distribution plans or golden output. - Use the existing Parquet statistics interface and physical row-location representation. Keep the TopN scheduling and pruning invariants. - Include the shared Arrow array normalizer required by the new reader, taken from the same master revision. Preserve 4.2 Lance/Avro dispatch; omit the schema-inference test for the unavailable Hive OpenCSV parser. ### Validation - FE production and test sources compile with the branch's Java 8 target. Across the targeted runs and focused reruns, 20 test classes report 181 discovered, 171 passed, zero failures/errors, and 10 skipped: nine intentionally disabled known-bug reproductions plus the opt-in live ES 9 class. - FE Checkstyle: passed with zero violations. - Clang compiler syntax checks: all 23 affected C++ translation units passed against the current sources and freshly generated Thrift, using cached third-party/generated build dependencies. Focused BE runtime validation is listed below. - clang-format 16: all 30 affected C++ source/header files passed. - Groovy 4.0.19 syntax and AST conversion: all seven changed suites passed. All 105 local inventory links resolve. - Complete staged-diff self-review, whitespace/conflict checks, and sensitive-information checks passed. A full BE rebuild and SQL/external-service regressions were not run locally. Live ES 9 tests require their opt-in endpoint. CI is requested for the combined backport. ### Release note Fix Paimon reads after field renames, external-catalog SHOW DATABASES authorization, and ES compound residual filtering. Support ES 9.x node discovery, publish Iceberg Parquet split offsets, improve Parquet V2 TopN row-group scheduling, and use FileScannerV2 for Arrow loads and text/JSON schema inference. ### Check List (For Author) - Test - [x] Unit Test - [x] Regression test additions and syntax checks; SQL execution pending CI - Behavior changed: - [x] Yes. Backport the changes listed above. - Does this need documentation? - [x] No. Existing interfaces and settings are retained; the test coverage inventory is included. ### Follow-up fixes - Set the Thrift `enable_file_scanner_v2` default to `true`, matching the session default. Preserve explicit `false` for legacy query scans and extend the existing load-selection test. - Keep the selected Paimon snapshot and schema together during relation projection. Historical sequence/aggregation options must not be combined with the latest field names. Read-only principals can resolve historical selectors without INSERT permission. - Read legacy append schemas with missing or empty bucket keys without bucket pruning, preserving every historical file. The regression covers both unaware and multiple fixed buckets. Follow-up validation: 169 targeted Paimon FE tests passed with zero failures/errors/skips. FE production compilation and Checkstyle passed. Both changed C++ test translation units passed compiler syntax checks and clang-format 16. Regenerated Java/C++ defaults are enabled; a Java 8-targeted protocol check verifies constructor/clear defaults, an omitted wire field, and an explicit-false round trip. Complete BE CI and live external SQL suites remain pending. The two new reproductions first failed with the same missing-bucket-key and renamed-sequence-field errors as CI. A separate read-only-principal reproduction also failed before the permission correction. Complete external regression execution remains pending CI. ### BE unit-test compatibility - Complete the native Arrow Variant input dependency for `branch-4.2`: accept binary storage and the Parquet Variant extension, validate metadata and payloads before appending, and retain owned data after Arrow buffers are released. Allow that ingestion path through the load reader while preserving query-format restrictions and source-default validation. - Preserve Variant V2 markers in the Arrow test fixtures, including nested types. Verify timestamp input against this branch's SerDe for positive, negative, and second-boundary values. Match the existing DATE range diagnostic in the TopN pruning test. - Add tests for sliced and empty reads, invalid ranges and carriers, malformed metadata/payloads, NULL children, atomic failure, buffer ownership, and the query capability boundary. Validation: all 13 previously failing BE tests passed. The expanded run passed all 309 tests across nine suites, including Arrow/JSON loads, load validation, Iceberg, table mapping, and Variant input/output. The focused executable rebuilds the affected readers and SerDe against cached 4.2 build dependencies; this is not a complete BE rebuild. All seven changed C++ translation units passed compiler checks with current headers/generated Thrift, and all 33 affected C/C++ files passed clang-format 16. The complete staged diff, whitespace, and sensitive-information checks passed. Full CI is requested with `run buildall` and `/review`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
After renaming a Paimon sequence or aggregation field, current and historical reads can fail with a missing-field error. Paimon's table copy selects the historical columns but overlays current table options, so
sequence.fieldandfields.<column>.aggregate-functioncan reference names that do not exist in the selected schema.JNI transport also rebuilds tables without their catalog loader. The SDK factory can reapply a snapshot selector during that rebuild, reverting a schema-only rename before the bound schema is restored. Build with selectors temporarily removed and then restore the complete bound schema.
Restore the selected schema and its physical options together before applying query options. Preserve the resolved table's read-state keys and branch policy, including absent keys, so historical options cannot resurrect a removed scan mode or change fallback priority during parallelism normalization. Preserve explicit catalog overrides and the SDK's fallback-branch snapshot translation. Apply the same logic to scan planning, metadata, statistics, and the source used to rebuild system-table wrappers, so binding and JNI serialization agree.
Add real-table tests for snapshot, timestamp, tag, and OPTIONS reads after renames, current reads after schema-only renames, catalog-loader removal and serialization, catalog override precedence, fallback branches, empty-table timestamps, and read-optimized system tables. Add an end-to-end regression covering sequence semantics and repeated aggregation-field renames with
force_jni_scanner=falseandtrue.Release note
Fix Paimon current and historical reads after renaming sequence or aggregation fields.
Check List (For Author)