Static review of #69493 at 8fd3bec. Six verified P2 findings are inline: stored-NULL error suppression, missing TIMESTAMP_NS preflight, reserved materializer names, an overbroad nested-distance exemption, retained ORC/LTZ files, and loss of literal-string identity in the shared regression oracle.

Critical checkpoints:

1. Goal and behavior: the SDK upgrade and PK-vector ANN path are connected from Nereids through split serialization to Rust. The six findings prevent full correctness/compatibility; new tests do not cover these inputs.
2. Scope and clarity: changes span all 61 authoritative paths; the shared eligibility refactor is relevant. The regression helper affects comparisons beyond ANN and needs the inline correction.
3. Concurrency: no new threads or lock order; Backend capability fields are volatile and heartbeat/replay updates were traced. Scan-local and split-local ownership examined; no additional concrete race found.
4. Lifecycle/static initialization: C ABI setters copy inputs, filters transfer ownership, returned streams own required state, and RAII handles/Arrow callbacks cover examined error paths. No new cross-translation-unit static dependency identified.
5. Configuration: existing Rust-reader and FileScannerV2 session switches gate the rewrite; no new dynamic configuration item.
6. Compatibility: Java Paimon 1.4.2 -> 2.1.0 and Hive shade 3.1.3 -> 4.1.0 reviewed, including VECTOR/JNI physical layout, removed privilege wrapper, S3 options and retained schema/file compatibility. Inline findings cover remaining proven input-shape failures.
7. Thrift: touched definitions compared with master and branch-4.2/4.1/4.0/3.1. Added IDs 12/21/6/42 avoid collisions, including master reservations 20/41. Required members are inside a new optional, capability-gated payload type; no old-sender required-field failure demonstrated.
8. Parallel paths: batch Rust, vector Rust, JNI/native fallback, logical/physical/lazy scan copies, filesystem/HMS/JDBC and relevant REST/DLF paths reviewed. New vector plans have no fallback, which makes the reported preflight gaps observable failures.
9. Conditions: predicate whitelists, NULL query elements, IN bounds, incremental/file-time exclusions, branch/snapshot identity and pinned-empty state traced. Canonical field casing is preserved; apparent lowercasing issues were dismissed from actual descriptor code.
10. Tests: inspected FE/BE unit and differential regression additions; no builds or tests executed under the static-only contract. Missing cases are stated inline. Profile presence alone is not proof of vector execution.
11. Expected results: inspected changed temporal/date, table-property and oversized-row expectations; no additional proven expectation defect. The new shared binary normalization can hide mismatches (inline).
12. Observability: ANN explain fields, fallback reason logging and Rust timers are present. No additional critical logging/metrics defect established; large-K behavior was not benchmarked.
13. Transactions/persistence: no changed write/commit/EditLog protocol. New heartbeat capability serialization and replay/default-false downgrade behavior checked.
14. Data writes/crashes: production changes are read/planning paths; no new table mutation or transaction atomicity change. Reader cleanup/error propagation inspected statically.
15. FE/BE state: AnnTopNInfo survives logical/physical copies and translation; query-wide request and per-bucket bytes reach the Rust reader. Old BEs are excluded by independently negotiated vector support.
16. Performance/memory: source reads stream into bounded search heaps; selected TopK payloads are eagerly materialized and ignore ordinary batch-size controls. Static inspection does not establish a separate lifetime/tracker defect; runtime memory/cancellation and dependency-linkage validation remain outside this review.
17. Other concerns: the whole PR and upstream/downstream special cases were checked beyond the first finding; candidate dispositions and risk coverage are recorded in the shared ledger.

User focus: no additional focus was supplied. Existing thread 4357063534 about reusing the heartbeat flag was reviewed as a duplicate fence and not reposted; no existing P0/P1 blocker was independently confirmed. Coverage limits: static source review only, with no build, test, external-service execution or performance measurements. No repository source changes or submodule initialization were performed.

Review converged after 2 round(s).

Reviewed #69493 at 199ad37. Request changes for two new P1 findings (regression-framework syntax and unaccounted eager vector materialization) and one P2 S3 credential-precedence regression.

Critical checkpoint conclusions:

1. Goal and proof: the SDK upgrade and PK-vector ANN path are implemented across planner, split transport and BE; focused FE/BE tests and differential regression cases are present, but this static review does not prove runtime success.
2. Scope: the production changes form a coherent feature/SDK migration; shared regression normalization expands the affected surface and contains the syntax finding.
3. Concurrency and locks: query planning and scanner-local state introduce no new shared lock order; Rust executes through its Tokio runtime. Its unaccounted eager allocations are covered by the memory finding.
4. Lifecycle: table reuse keys include schema/options/branch, split cleanup follows ownership order, and the pinned eager API returns an independently owned Arrow stream before local handles are freed. No additional lifetime defect established.
5. Configuration: existing Rust-reader and FileScannerV2 session switches gate the feature; no new dynamic server configuration is introduced.
6. Compatibility: VECTOR is consistently mapped to Doris ARRAY with the corresponding JNI physical accessor. Unsupported reader shapes mostly decline before the one-way rewrite; existing NULL-vector and retained-ORC concerns remain below.
7. Thrift: touched definitions were compared on HEAD, branch-4.2, branch-4.1, branch-4.0, branch-3.1 and master. New parent fields do not reuse IDs; master's reserved fields20/41 are avoided. Required children belong to a wholly new optional payload, so old readers skip that payload.
8. Parallel paths: normal batch/raw/JNI paths, Hudi/lazy scan copies and both metrics were checked. The shared filesystem/HMS/JDBC S3-option mirror overwrites explicit provider options, as reported inline; generated Hadoop defaults must preserve explicit catalog choices.
9. Conditional checks: filter shapes/types/NULL literals/IN size, snapshot/branch/options/empty generation, nested projection, nanoseconds and reserved names were traced through callers and consumers. Downstream TopN analysis rejects limit overflow; runtime filters do not cross the retained TopN.
10. Test coverage: positive and negative planner cases, wire/schema guards, score conversion, partition metadata, filter fallback, assertions and both metrics are represented. No eager-memory admission test covers the new resource defect.
11. Changed expected results: timestamp/date, property-default and error-prefix changes were inspected; no golden result files were edited. Framework syntax must be repaired before these suites can run.
12. Observability: existing reader open/read/Arrow timers cover execution, and fallback-reason logs explain dispatch without logging credential values. No additional observability blocker established.
13. Transactions and persistence: production EditLog, commit and failover logic are unchanged; relation snapshot selection and pinned-empty fencing were traced.
14. Writes and atomicity: production write/transaction paths are unchanged; the adjusted write-failure test still checks rows and snapshot state after failure.
15. FE-BE variables: AnnTopNInfo survives logical/physical copies and translation; query-wide search parameters and per-split payload agree with BE consumption.
16. Performance and memory: table-handle reuse and merge-free compacted batch reads are retained. Eager all-K payload collection/reordering bypasses adaptive batch sizing and tracked admission, as detailed inline.
17. Other checks: source casing prevents the suspected ordinary score-name misrouting, and small f32 score/tie differences were not raised as a separate defect under the approximate ANN contract.

Existing review context was revalidated and not reposted. The stored-NULL-vector validation issue in discussion4357228424 still applies. Discussion4357228441 also remains applicable to per-level file formats: a Parquet default plus file.format.per.level='0:orc' can retain ORC/LTZ files that the preflight misses. Its original schema-history trigger is now guarded. Nanosecond projection (4357228428), reserved vector materializer names (4357228431), distinct distance-call exemptions (4357228435), and the redundant heartbeat capability (4357063534) are addressed. Generic string/binary normalization ambiguity remains covered by4357228447; the syntax failure reported here is distinct. No existing P0/P1 inline finding required carrying forward.

User focus: no additional focus points were supplied. All 55 authoritative changed paths and relevant upstream/downstream contracts received static coverage. Builds, tests, live query execution and SDK interoperability matrices were not run, as required by the review contract; no source files or submodules were modified.

Review converged after 3 round(s).

Request changes on exact head af5c7cc. Four new verified findings are inline: legacy Paimon authorization is silently removed, legacy serialized privilege tables break BE-first upgrades, vector scans bypass cross-storage file eligibility, and non-finite query vectors change supported queries into errors.

Existing blocking finding 4358943088 still applies: uncapped LIMIT+OFFSET reaches eager Rust materialization without tracked admission or a byte bound. It is carried in existing_blocking_comment_ids rather than reposted. The previous Groovy quoting P1 is fixed by the slashy literal; the explicit S3-provider overwrite is fixed by filling only absent options. Existing stored-NULL-vector, per-level ORC/LTZ eligibility and literal-comparator ambiguities remain covered by their existing threads and are not duplicated. Reserved-name, projected nanosecond timestamp and exact distance-call exemption guards are present.

Critical checkpoint conclusions:

1. Goal and proof: the PR adds the complete Nereids -> Java bucket planning -> Rust ANN pipeline and focused tests, but the four findings prevent accepting its behavior as complete.
2. Scope: the SDK, VECTOR/JNI mappings, credentials and plan plumbing changes were reviewed together across all 56 changed paths.
3. Concurrency and locks: immutable ANN plan data is copied defensively; no new Doris thread entry or lock-order change was identified. Rust asynchronous work remains SDK-owned, including the already-reported untracked allocations.
4. Lifecycle: split/table handles, early error cleanup, Arrow ownership, nullable score conversion and query-homogeneous split reuse were traced; no additional lifetime or static-initialization defect was verified.
5. Configuration: existing reader/scanner session switches are reused; no new dynamic Doris configuration was introduced.
6. Compatibility: both supported BE-first serialized-table compatibility and fully upgraded catalog authorization have new regressions, reported inline. The reverse-order v9 split concern was dismissed using the documented BE-before-FE upgrade order.
7. Thrift/protobuf: touched Thrift definitions were compared with branch-3.1, branch-4.0, branch-4.1, branch-4.2 and master. New outer fields are optional and avoid master IDs 20/41; no conflicting ID reuse or required addition to an existing struct was found. No protobuf change.
8. Parallel paths: ordinary native/JNI/Rust readers, vector readers, all scan-copy/translation paths, Hudi constructor adaptation and filesystem/HMS/JDBC catalog variants were covered.
9. Conditional gates: unsupported filter shapes, NULL query literals, oversized IN, data predicates without the required DV mode, projection and schema-history guards were checked. Cross-storage files and non-finite queries are the new missing preflights.
10. Tests: FE/BE unit tests and external regression suites were inspected for positive, negative, partition, snapshot, schema and metric cases; no builds or tests were executed.
11. Expected results: changed expectations and error assertions were reviewed statically; no generated .out file was modified. This review does not certify runtime output.
12. Observability: ANN explain fields and reader timers exist; fallback reasons are logged once per combination. The regression profile assertion only checks PaimonRustReader, which also exists for ordinary Rust scans and cannot alone prove ANN executed.
13. Transaction/persistence: no Doris EditLog, transaction-commit or persistence-format modification was found; statement snapshot/branch/pinned-empty handling was traced.
14. Writes/atomicity: direct changes are on read/planning paths; no new Doris data-write atomicity change was found. The dependency upgrade's authorization effect is reported rather than assumed safe.
15. FE/BE propagation: vector column, f32 bytes, metric, retrieval K, synthetic output and per-bucket payload were traced through logical/physical copies, translation and decoding.
16. Performance/memory: the existing eager-materialization P1 remains blocking. The relaxed ordinary compacted-file gate was checked against Java split guarantees and Rust dispatch; no additional losing-batch retention trigger was verified.
17. Further recheck: predicate canonicalization, runtime-filter boundaries, branch/options identity, external file storage, legacy privilege metadata, Java serialization and ordinary score-named columns were rechecked; every candidate has an accepted, existing-thread or evidence-based dismissal disposition.

User focus: no additional focus was supplied; the complete PR was reviewed. Coverage limits: static analysis only, with no live query, build, test or benchmark validation. No unresolved review candidate or uncovered changed-file scope remains. Convergence describes review stability; the new authorization P1 and existing memory P1 still require changes.

Review converged after 2 round(s).

Existing P0/P1 findings confirmed for this head: #69493 (comment)

Request changes on head 9544d3a.

Three new inline findings are verified: [P1] captured TVF prefilters bypass required comparison normalization; [P2] prefilter inputs can be removed by later column pruning while the request still references them; [P2] TPaimonFileDesc field 21 collides with branch-4.1 blob_files. The two prefilter findings concern different invariants: expression semantics and preservation of hidden scan dependencies.

Existing blocking findings independently confirmed on this head, without reposting:

• 4358943088: eager vector materialization still bypasses Doris allocation admission/accounting and retains projected Top-K batches before returning. The TVF row-count cap does not bound projected bytes; ordinary SQL still forwards unrestricted LIMIT+OFFSET. Rust 0.5 has ordinary-reader resource APIs, but this vector path does not integrate them.
• 4373642116: legacy warehouse authorization still applies: the 2.1 factory no longer supplies the legacy privilege wrapper, and the actual catalog initialization has no legacy-metadata migration/rejection guard.

The prior Groovy syntax P1 is fixed and is not carried. Existing threads already cover stored NULL-vector validation, old-FE privilege-wrapper deserialization during BE-first upgrades, and the remaining bare-token ambiguity in result normalization. Nonfinite query coordinates, explicit S3 provider precedence, reserved materializer names, distinct distance-call projection checks, nanosecond columns, retained ORC/LTZ eras and external-file eligibility now have the inspected guards; no duplicates are posted.

Critical checkpoint conclusions:

• Goal and proof: SQL/TVF ANN, shared Rust eligibility and 2.1/0.5 adaptation are implemented, but the verified prefilter errors prevent accepting the behavior. Added FE/BE/regression tests were inspected; they omit the raw comparison and post-pruning cases above.
• Scope and clarity: shared eligibility and typed ANN state keep the change coherent; the dependency upgrade also affects ordinary scans and authorization, which were included in coverage.
• Concurrency and locking: Paimon range conversion is serial on this branch; no new Doris thread, shared lock, nested lock order or heavy operation inside a new lock was introduced. SDK runtime ownership was traced.
• Lifecycle and static initialization: local FFI handles, cloned Rust read state, independent Arrow stream and split EOF/close paths were inspected; no new ownership cycle or cross-TU initialization issue was established. Hidden predicate dependency lifetime is the second inline finding.
• Configuration: existing Rust-reader and FileScannerV2 switches gate ordinary rewrites; no new dynamic configuration was added. Explicit TVFs require the supported reader configuration.
• Compatibility: the field-ID collision and existing authorization/serialization upgrade concerns remain. The two required members in the new optional payload also conflict with the shared-definition optional-field convention; this is noted with the protocol finding.
• Thrift/proto: every touched definition was compared with live master and branches 4.2, 4.1, 4.0 and 3.1. Request field 6 and scan-params field 42 avoid the inspected allocations; payload field 21 does not. No proto changes.
• Parallel paths: ordinary JNI/native/Rust, SQL ANN, Paimon/Lance TVFs, prepared rebinding, Hudi scan constructors, provider-specific row-ID materialization and filesystem/HMS/JDBC S3 initialization were inspected.
• Special conditions: predicate conversion, partition/data separation, global offset, runtime-filter TopN barriers, snapshot/tag/branch identity, empty snapshots, nested projection, schema history, VARIANT and timestamp gates were traced. No unresolved risk-focus question remains.
• Functional and negative testing: inspected positive/negative FE tests, BE conversion/protocol tests and Spark-produced external regression fixtures. No build, test, benchmark or end-to-end query was executed under the static-only contract.
• Changed results: no golden .out file changed; regression expectations, error assertions and numerical comparisons were inspected. Their runtime correctness is not claimed.
• Observability: ANN metric/limit explain output, distinct fallback reasons and existing open/read/convert timers supply routing context; allocation observability remains limited with the known memory P1.
• Persistence and transactions: no Doris EditLog, publish-version or transaction-state modification; bound external snapshot/schema transport was inspected. No new failover/persistence defect was established.
• Writes and crash behavior: production additions are read planning/materialization; write regression expectations changed. No new write-atomicity change was found; unaccounted eager allocations remain an existing process-memory risk.
• FE/BE variables: query vector, metric, candidate count, provider options and split payload were traced through the logical/physical/Thrift/reader chain; hidden prefilter slot loss and protocol allocation are the reported boundary defects.
• Performance: eager projected materialization remains blocking. Raw-split/merge dispatch, per-split state reuse and batch boundaries were inspected; no additional substantiated performance regression was found.
• Further sweep and user focus: no additional user focus was supplied. Casing and fallback-log concurrency suspicions were dismissed with caller/descriptor evidence; every candidate and risk item has a disposition.

Convergence: converged after 2 rounds. All three reviewers returned NO_NEW_VALUABLE_FINDINGS in round 2; all 70 changed paths and assigned mechanisms were covered, and the main review independently verified and deduplicated every accepted candidate. No unresolved static-review candidate or uncovered assigned scope remains. Runtime interoperability, allocator behavior and test results remain unexecuted static-review limits, not claimed validation.

Review converged after 2 round(s).

Existing P0/P1 findings confirmed for this head: #69493 (comment), #69493 (comment)