[fix](iceberg) Allow optional position delete fields without nulls

[Gabriel39]

Proposed changes

Relax Iceberg position delete Parquet reads so Doris can consume delete files where file_path or pos are declared optional in Parquet metadata but contain no actual null values.

The reader now materializes position delete columns as nullable, validates that each batch has no nulls, then strips the nullable wrapper before building delete ranges. Actual null values in file_path or pos still fail with a corruption error.

This covers the legacy Iceberg Parquet reader, the shared delete-file helper used by Iceberg writes, and the format_v2 position delete collector.

Problem

Some writers, including DuckLake/DuckDB-produced data seen in the field, can produce Iceberg position delete files whose required Iceberg fields are encoded as optional Parquet fields. Doris previously rejected these files before checking whether the data actually contained nulls.

Testing

• git diff --check
• JDK_17=/usr/local/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home JAVA_HOME=/usr/local/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home ./run-be-ut.sh --run --filter='IcebergDeleteFileReaderHelperTest.*'
  • Blocked locally during CMake configuration because this checkout is missing ANN/vector-search prerequisites (contrib/openblas, contrib/faiss, OpenMP).
  • A prior run without an external DORIS_THIRDPARTY also blocked before compiling due missing thirdparty/installed/bin/protoc and Snappy.

[hello-stephen]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[Gabriel39]

/review

[Gabriel39]

run buildall

[github-actions]

Automated review result: request changes.

I found one issue: the PR changes the standalone delete-file helper plus the v1 and format_v2 scan-reader position-delete paths, but the new tests only exercise the helper. Please add reader-path coverage for nullable file_path/pos delete files, including no-null success and actual-null rejection through prepare_split / get_block.

Critical checkpoint conclusions: the code is narrowly scoped to Iceberg position-delete nullable handling; no new concurrency, lifecycle, config, persistence, FE/BE protocol, or observability surface was introduced; error propagation is improved by returning Status from _gen_position_delete_file_range; performance impact appears limited to null-map checks on delete-file batches. The only blocking gap I found is test coverage for the parallel reader paths changed by this PR.

Validation: static review only. This checkout is shallow and lacks .worktree_initialized, thirdparty/installed, thirdparty/installed/bin/protoc, and a built doris_be_test, so I could not run the BE unit test. I did run git diff --check and clang-format-16 --dry-run --Werror on the changed C++ files successfully.

User focus: no additional user-provided review focus.

Subagent conclusions: tests-session-config proposed TSC-1, accepted as the inline comment. optimizer-rewrite reported no optimizer/rewrite, join, aggregate, or delete-application semantic issue. In convergence round 1, after M-1 was accepted and the final comment set contained only M-1, both live subagents replied NO_NEW_VALUABLE_FINDINGS.

[Gabriel39]

/review

[Gabriel39]

run buildall

[github-actions]

Codex automated review failed and did not complete.

Error: Review step was skipped (possibly timeout or cancelled)
Workflow run: https://github.com/apache/doris/actions/runs/28995281494

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

[hello-stephen]

BE UT Coverage Report

Increment line coverage 52.94% (36/68) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	56.54% (23209/41051)
Line Coverage	40.10% (225231/561732)
Region Coverage	36.07% (178006/493501)
Branch Coverage	37.09% (79057/213135)

[Gabriel39]

/review

[github-actions]

Static review completed for PR 65401.

I did not find any new non-duplicate correctness issue to submit as an inline comment. The one substantiated remaining concern I found is already covered by the existing inline thread on be/test/format/table/iceberg/iceberg_delete_file_reader_helper_test.cpp: reader-path coverage for nullable Iceberg position-delete schemas, especially the legacy/v1 IcebergParquetReader path. Current head adds format_v2 reader-path tests for nullable no-null, null file_path, and null pos, while the remaining v1 coverage request is already in that thread, so I am not duplicating it.

Critical checkpoint conclusions:

• Goal and tests: the code changes consistently read Parquet position-delete file_path/pos columns as nullable, reject actual nulls, strip nullable wrappers, and propagate errors through helper, v1 reader, and format_v2 collector paths. Helper and format_v2 tests were added; v1 reader-path coverage remains the already-open review-thread item.
• Scope: the implementation is focused on Iceberg position-delete readers and related tests.
• Concurrency/lifecycle: no new thread, lock, static-initialization, or ownership lifecycle risk found. Existing delete-file cache use remains unchanged except that parse errors are now propagated from _gen_position_delete_file_range.
• Configuration/compatibility/protocol: no new config, persisted format, Thrift, or FE-BE protocol change. This is a reader-side compatibility relaxation for optional Parquet physical schemas without accepting actual null Iceberg values.
• Parallel paths: helper, legacy/v1 Parquet reader, ORC call-site status propagation, and format_v2 Parquet collector were inspected. The remaining test gap for the v1 reader path is already covered by the existing thread.
• Error handling/null handling: actual nulls return Status::Corruption with the offending column name; callers now check the returned Status. Nullable columns are unwrapped only after has_null(0, read_rows) succeeds.
• Performance/observability: no material new hot-path issue found; the added null scan is per delete-file batch and error messages identify file_path or pos.
• Test result review: git diff --check passes. I did not run BE unit tests because this checkout is missing .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc.
• User focus: no additional user-provided review focus was present.

Subagent conclusions:

• optimizer-rewrite: no candidates; convergence round 1 returned NO_NEW_VALUABLE_FINDINGS.
• tests-session-config: no candidates; it independently noted the only remaining coverage concern is duplicate of the existing thread; convergence round 1 returned NO_NEW_VALUABLE_FINDINGS.
• Final proposed inline comment set for this pass: empty.

[Gabriel39]

run buildall

[Gabriel39]

/review

[github-actions]

Automated review summary for PR #65401 at 87ea629.

I did not find any new source-level issue that should be submitted as an additional inline comment. The nullable position-delete reader-path coverage concern is already present in the existing inline thread discussion_r3548953477, so I did not duplicate it.

Critical checkpoint conclusions:

• Goal: the PR relaxes Iceberg Parquet position delete reads so optional file_path/pos fields without actual nulls are accepted, while actual nulls fail. The production changes implement this by reading nullable columns, checking batch null maps, and stripping the nullable wrapper before the existing string/dictionary/int64 processing.
• Scope: the patch is focused on Iceberg position-delete helper, v1 reader path, format_v2 reader path, and related BE tests.
• Concurrency/lifecycle: no new shared mutable state, threads, locks, globals, or static initialization concerns were introduced.
• Configuration/session/persistence/protocol: no new config, session variable, persisted format, or FE/BE protocol field is added.
• Compatibility: the behavior change is a compatibility relaxation for Parquet schemas that mark Iceberg-required delete fields optional. Actual null values still return corruption errors; ORC behavior is effectively unchanged except status propagation from the shared v1 helper.
• Parallel paths: helper-level reads, v1 Parquet/ORC position-delete range generation, and format_v2 collection were reviewed. Status propagation from _gen_position_delete_file_range is now checked by both v1 callers.
• Tests: helper-level nullable Parquet tests and format_v2 reader-path nullable tests are added. The remaining v1 reader-path coverage ask is already covered by the existing inline review thread, not repeated here.
• Test results: I did not run BE UT locally because this runner lacks .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc. GitHub BE UT (macOS) failed before Doris compilation with ERROR: The JAVA version is 25, it must be JDK-17, so I did not treat that as a source-level failure from this patch. Added-line trailing whitespace check over the GitHub patch was clean.
• Observability/errors: the new corruption messages identify the offending delete column (file_path or pos), which is sufficient for this path.
• Data correctness: position deletes remain version/path based as before; the change only affects physical nullability handling while preserving null rejection.
• Performance: the added per-batch null-map scan is bounded by the delete batch size and avoids per-row null branching after validation.

Subagent conclusions:

• optimizer-rewrite: no optimizer/rewrite, parallel join, or aggregate-path candidate found.
• tests-session-config: no distinct evidence-backed test/config/style candidate found beyond existing thread discussion_r3548953477.
• Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger and empty proposed inline comment set.

User focus: no additional user-provided review focus was present.

[github-actions]

PR approved by at least one committer and no changes requested.

[hello-stephen]

BE Regression && UT Coverage Report

Increment line coverage 61.76% (42/68) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	74.85% (29993/40072)
Line Coverage	58.90% (328997/558577)
Region Coverage	55.65% (275953/495863)
Branch Coverage	56.97% (121570/213392)

1

[github-actions]

PR approved by anyone and no changes requested.