[fix](be) fail the query instead of crashing BE on iceberg schema-mapping mismatch

[u70b3]

What problem does this PR solve?

Issue Number: #61225 — intentionally NOT auto-closing. Pattern B is root-caused at the crash mechanism level, and Pattern A is now fully guarded at the crash site (any truncated or out-of-bounds dictionary index stream returns Status::Corruption), but the upstream reason a data page can reference a missing/corrupt dictionary remains unproven because the issue has no reproducer. The issue should stay open until that is confirmed.

Related PR: cbfe309 (precedent: has_children_column guard for the top-level parquet path), #65784

Problem Summary:

BE crashes while scanning Iceberg tables in the two modes reported by #61225:

1. Pattern B – std::out_of_range / DCHECK abort. StructNode::children_column_exists does a bare children.at(). If the schema info from FE is inconsistent with the scan projection, the whole BE process dies (release: uncaught exception; debug: DCHECK abort). The top-level parquet entry was fixed by cbfe309, but sibling call sites were not:

   • OrcReader::_do_init_reader top-level missing-column loop (the ORC twin of cbfe309)
   • IcebergParquetReader/IcebergOrcReader::on_before_init_reader classification loops (PARTITION_KEY + REGULAR branches)
   • Nested struct field resolution: StructColumnReader::read_column_data (parquet) and OrcReader::_fill_doris_data_column (orc)
   • IcebergPositionDeleteSysTableReader ($position_deletes system table)
   • Filter/optimization paths that iterate all tuple slots: page-index stat func, min-max/bloom lambdas, bloom cache backfill, dict-filter fallback in group reader, ORC pushdown type checks. These touch every slot in the tuple — including synthetic slots such as TopN's GLOBAL_ROWID_COL that are never registered in the schema tree — so they can crash even without any FE misbehavior.

2. Pattern A – SIGSEGV. ByteArrayDictDecoder::_decode_values indexes an empty _dict_items when data pages reference a dictionary that was never decoded, dereferencing a null StringRef.

This PR applies defense-in-depth using the has_children_column API from cbfe309:

• Query contract paths fail loudly with Status::InternalError — a contract violation is a bug and must not produce silent wrong results.
• Optimization/filter paths silently skip the optimization (return false / fall back to plain conjunct filtering) — no result correctness impact.
• Pattern A is guarded at the crash site, not root-caused: the decoded dictionary index stream is now fully validated before use — GetBatch must return exactly the requested count (truncated stream → Status::Corruption) and every index must be smaller than the dictionary size (out-of-range index, including the empty-dictionary case → Status::Corruption). This also covers the dictionary-column path, whose raw indices previously crashed later in convert_dict_column_to_string_column. The issue provides no reproducer, so the exact reason a dictionary page can go missing remains unproven; this converts every SIGSEGV mode at that stack frame into a query error.

The schema mapping built by FE is designed as a superset of the BE scan slots (top-level names = requested columns, row-lineage fields appended explicitly, time-travel/TopN use the full schema, partition-evolution PARTITION_KEY columns are present), so the new guards only fire on a genuine contract violation and never change legitimate query behavior.

Release note

Fix BE process crash (std::out_of_range / SIGSEGV) when scanning Iceberg tables with inconsistent schema mapping - the query now fails with an error instead.

Check List (For Author)

• Test

  • Unit Test — (a) decoder level: empty dictionary with non-null data, out-of-range dictionary index (plain and dictionary-column paths), and truncated index stream all return Status::Corruption (byte_array_dict_decoder_test.cpp); (b) reader level: IcebergParquetReader init fails with InternalError when the FE schema info misses a projected column (iceberg_reader_test.cpp), and page-index filtering skips a synthetic TopN GLOBAL_ROWID_COL slot that the FE-built schema tree never registers (parquet_expr_test.cpp); (c) helper level: HasChildrenColumnGuardsNestedStructField / NestedStructFieldMissingInFileKeepsKey. Reader-level coverage for a known nested field absent from the file already exists (v2_parquet_materializes_nested_initial_default_without_reviving_parent, v1_top_level_missing_binary_prefers_iceberg_initial_default). All 130 tests in the touched suites pass locally; the external_table_p0/p2 Iceberg regression suites cover behavior invariance.

• Behavior changed:

  • No. (Legitimate query behavior is unchanged. On a schema-mapping contract violation, the query now fails with InternalError instead of crashing the BE process.)

• Does this need documentation?

  • No.

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

Thanks for the defense-in-depth work. I have two concerns that should be addressed before merge:

1. The new tests do not exercise the production fixes. HasChildrenColumnGuardsNestedStructField and NestedStructFieldMissingInFileKeepsKey only validate the existing has_children_column() helper state. They would still pass if every new reader guard and the ByteArrayDictDecoder change in this PR were removed. Please add direct regression coverage for:

   • an empty byte-array dictionary with non-null dictionary-encoded values returning Status::Corruption instead of crashing;
   • a Parquet/ORC reader schema-contract mismatch returning Status::InternalError;
   • a known nested field that is absent from the file still materializing its default or NULL at reader level.

   An SQL reproducer is not required for these cases; the existing reader/decoder unit-test fixtures can construct the inconsistent state directly.

2. close #61225 seems premature for Pattern A. The PR explicitly says that the root cause is unproven and currently only guards the _dict_items.empty() case. The reported stack location alone does not establish that this was the exact state that caused the original SIGSEGV. Please either add a reproducer/test that ties the reported failure to an empty dictionary, or describe this part as hardening/partial mitigation and avoid automatically closing the entire issue until the root cause is confirmed.

The has_children_column() short-circuit ordering, the distinction between query errors and optimization fallback, and preservation of the known-but-file-missing path otherwise look reasonable.

[u70b3]

Review summary

I recommend requesting changes. This static review found three issues that should be addressed before merge.

[P1] Reduce defensive changes that lack a reproducer or a proven reachable call path

Pattern B in issue #61225 reaches the top-level Parquet initialization path. On this PR's base, that location was already guarded with has_children_column by cbfe3096dff. This PR expands the change to ORC, nested structs, $position_deletes, page-index filtering, min-max filtering, bloom filtering, and dictionary-filter fallback. These are several independent paths, but the PR does not demonstrate, for each one, a reachable call chain from #61225 or a regression test that exercises it.

This is also in tension with the repository's coding guidance: error checks should correspond to a known, inevitable failure path rather than a speculative defensive if. Most of these guards appear to come from enumerating potential call sites instead of demonstrating an actual failure path.

Suggested changes:

1. Keep only Pattern B call sites backed by a reproducer, stack trace, or a clearly demonstrated reachable path.
2. Move TopN synthetic-slot handling, ORC, $position_deletes, and other independent cases into separate changes with dedicated tests.
3. If the goal is to eliminate children.at() hazards systematically, prefer a centralized safe lookup API instead of repeating has_children_column() + children_column_exists() across many call sites.

[P1] Pattern A only checks for an empty dictionary, so out-of-bounds access remains possible

The new check in byte_array_dict_decoder.cpp only covers _dict_items.empty(). The code that follows still:

• does not verify how many indices GetBatch() actually decoded;
• does not verify that every decoded index is smaller than _dict_items.size();
• directly evaluates _dict_items[_indexes[...]] on the regular decoding path;
• stores unvalidated indices on the dictionary-column / dictionary-filter path, where later string conversion can still index out of bounds.

For example, if the dictionary contains two entries and a data page contains index 3, the dictionary is non-empty, so the new guard is bypassed and the BE can still crash. The PR explicitly says that Pattern A has no reproducer and its root cause is unproven, so the current stack trace is not enough to conclude that the dictionary must be empty.

After GetBatch() and before entering either decoding branch, please validate that:

1. the returned count equals non_null_size;
2. every decoded index is smaller than the dictionary size;
3. failures return Status::IOError or Status::Corruption.

The V2 reader already has a suitable decoded-count and index-bounds validation implementation that can be used as a reference.

[P2] The added tests do not exercise the production call sites changed by this PR

The two new tests in table_schema_change_helper_test.cpp only call BuildTableInfoUtil, has_children_column(), and children_column_exists(). Those APIs and behaviors already existed before this PR. Removing every new guard added to the Parquet and ORC readers would still leave these tests green.

Please add at least:

• reader-level tests that construct a schema tree missing a top-level or nested key and assert InternalError instead of an exception or abort;
• an optimization-path test containing a synthetic slot and asserting that the optimization is skipped and normal filtering is used;
• ByteArrayDictDecoder tests covering an empty dictionary, an out-of-range index, and a truncated index stream.

---

This review was generated by OpenAI gpt-5.6-sol from static code analysis. As noted in the task, the current checkout cannot be used for direct compilation, unit testing, or regression testing, so these findings do not include local runtime verification.

[u70b3]

Thanks for the reviews — all points are addressed in 961e933 (just pushed).

Pattern A validation (P1) — _decode_values now validates the decoded index stream before entering either decoding branch, mirroring decode_dictionary_indices in format_v2/parquet/reader/native/decoder.h: GetBatch must return exactly non_null_size indices (truncated stream → Status::Corruption), and every index must be < _dict_items.size() (out-of-range → Status::Corruption). This subsumes the previous empty-dictionary-only check and also covers the dictionary-column path, whose raw indices previously flowed unvalidated into convert_dict_column_to_string_column.

Tests (P2 / @Gabriel39 point 1) — direct regression coverage added:

• decoder level: empty dictionary + non-null data, out-of-range index (plain and dictionary-column paths), truncated index stream — all assert Status::Corruption (byte_array_dict_decoder_test.cpp);
• reader level: IcebergParquetReader init with FE schema info missing a projected column asserts Status::InternalError (parquet_init_fails_loudly_when_schema_mapping_misses_projected_column);
• optimization path: page-index filtering with a synthetic TopN GLOBAL_ROWID_COL slot absent from the FE-built schema tree asserts the optimization is skipped and the full row-group range is kept (test_page_index_filter_skips_synthetic_slot_absent_from_schema_mapping);
• "known nested field absent from the file still materializes its default/NULL at reader level" was already covered by v2_parquet_materializes_nested_initial_default_without_reviving_parent and v1_top_level_missing_binary_prefers_iceberg_initial_default.

All 130 tests in the touched suites pass locally.

Issue closing (@Gabriel39 point 2) — removed close #61225 from the description. Pattern A is now guarded at the crash site (every SIGSEGV mode at that stack frame returns Corruption), but the upstream trigger remains unproven without a reproducer, so the issue stays open.

Scope (P1) — kept as one PR because all guarded sites share a single mechanism (bare StructNode::children.at()), and the new synthetic-slot test demonstrates a reachable path that needs no FE misbehavior. A centralized safe-lookup API to replace the repeated has_children_column() + children_column_exists() pairs is a good follow-up; this PR intentionally keeps the cbfe309 precedent pattern for consistency.