Repository navigation
[feature](be) Add session controls for Parquet page indexes - #69971
Conversation
### What problem does this PR solve? Problem Summary: Parquet output currently relies on implicit page-index and data-page-size defaults. Add validated session variables for controlling ColumnIndex/OffsetIndex output and the target data-page size across OUTFILE, EXPORT, Hive and Iceberg writes. Preserve the existing enabled index default, 1 MiB page target and Hadoop-framed LZ4 compatibility limit. ### Release note Add enable_parquet_write_page_index (default true) and parquet_write_data_page_size (default 1048576 bytes) for new Parquet output. ### Check List (For Author) - Test: 16 BE ASAN tests and 9 FE tests passed; clang-format 16, build hygiene, FE Checkstyle and clang-tidy passed. - Behavior changed: Yes; users can explicitly disable page indexes and adjust the target data-page size. Existing defaults are preserved. - Does this need documentation: Yes; paired English and Chinese Parquet documentation.
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
TPC-H: Total hot run time: 28790 ms |
TPC-DS: Total hot run time: 151775 ms |
ClickBench: Total hot run time: 25.2 s |
FE UT Coverage ReportIncrement line coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
/review |
There was a problem hiding this comment.
No actionable defect was found in the five changed files at head 3b20906. No existing inline comments require retention, and no additional user focus was supplied.
The reviewed flow is:
SET -> SessionVariable -> forwarding/session snapshot -> toThrift/TQueryOptions
-> coordinator -> RuntimeState -> common VParquetWriter properties -> output file
EXPORT captures the session and runs generated OUTFILE statements through this flow.
Hive, Iceberg and native Parquet TVF sinks share the common writer.
Applicable critical-checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and proof | The two controls reach the common writer. Added FE tests inspect defaults, bounds and forwarding; BE tests inspect real metadata and readback. These tests were inspected, not executed. |
| Scope and reuse | The implementation is focused: existing session reflection, query-options transport, compression helper and Arrow property builder are reused. |
| Concurrency and locking | Values are captured into query options and read during per-file open. No new threads, shared mutable state, lock order or heavy locked operation is introduced. |
| Lifecycle and static initialization | Existing writer, converter and output-stream ownership/close paths remain. The new constant is constexpr; there is no cross-translation-unit initialization dependency or new resource cycle. |
| Dynamic configuration | Checked SET values affect subsequent statements without restart. Existing writers use their captured settings. Page size rejects zero, negative, nonnumeric and values above INT_MAX before assignment. |
| Format and rolling compatibility | Data-page/format versions, row-group policy and dictionary limit remain unchanged. Optional absent fields retain true/1048576 on new BEs; old BEs ignore the new fields and cannot implement the new controls. Defaults match the tagged Arrow 24 properties. |
| Thrift IDs | Read TQueryOptions on HEAD and live-verified branch-3.1, 4.0, 4.1 and 4.2. Added IDs 238/239 are unused on every release branch, optional, and have explicit defaults. The existing field237 mismatch between master and branch-4.1 predates this PR and is excluded from new findings. |
| Parallel paths | OUTFILE, EXPORT, Hive, Iceberg and native Parquet TVF writers receive the shared policy. Point-query and expression-only constant-folding paths were checked for relevance; no missing Parquet-writer sender was found. |
| Conditions and error handling | The index toggle is explicit; the documented Hadoop LZ4 branch preserves min(requested,131072). The positive-size assertion matches checked FE inputs and the nonzero Thrift default. No ignored Status or added exception-boundary issue was found. |
| Test coverage | Added tests cover index on/off, dictionary on/off, format versions, V1/V2 data pages, nullable/all-null values, metadata offset bounds, LZ4 target properties and Hive/Iceberg partition writers. FE tests cover valid/invalid bounds and map forwarding. SQL end-to-end and binary Thrift round-trip coverage are absent from the additions. |
| Expected test results | New counts, null counts and metadata expectations match the generated input by inspection. The FE test named ThriftSerialization constructs options; it does not execute a binary round trip. No golden output files changed. |
| Observability | Settings and output-file metadata expose the policy; existing error reporting remains. No new asynchronous lifecycle requires additional tracing or metrics. |
| Transactions and persistence | Generic annotated session persistence includes both primitive fields; missing saved keys retain defaults. EXPORT captures a deep clone and is cancelled on replay/failover. Transaction visibility, version and delete-bitmap paths are unchanged. |
| Writes, atomicity and crashes | File properties change within existing sink open/write/close and commit/abort protocols. No new ownership, crash-cleanup or atomicity defect was found. |
| FE-to-BE propagation | Both fields are set in common toThrift, marked needForward, copied through coordinators and RuntimeState, and consumed by common writer setup. Export worker contexts retain the captured session. |
| CPU and memory | Added setup work is constant per file and preserves the Arrow memory pool. Small and large page targets deliberately trade page overhead against buffering; size remains a target checked at batch boundaries, with Arrow's row limit retained. No concrete unintended performance regression was found. |
| Further recheck | Checked absent-index sequential fallback, page/header statistics, Iceberg split offsets, dictionary offsets, subclass overrides, export snapshots and extreme target handling. No unresolved static correctness candidate remains. |
BE header-hygiene review found no hub-header, PCH, explicit-instantiation, gate-table or source-.cpp test inclusion change. No new libc API or linker change is introduced.
Coverage is complete for the assigned static review. Builds, FE/BE tests, cluster SQL, mixed-version execution, external JVM-reader interoperability, stress tests and benchmarks were not run; the author's reported validation was not reproduced. These are validation limits, with no unresolved candidate withheld from the inline set.
Review converged after 2 round(s).
924060929
left a comment
There was a problem hiding this comment.
Parquet output currently uses shared writer defaults. This PR carries two session controls through TQueryOptions to the common writer, preserving default behavior and the Hadoop LZ4 page-size cap.
Independent static review of head 3b20906 found no actionable P0/P1/P2 defects within the FE scope and necessary FE/BE boundaries. Checked validation, SET_VAR restoration, FE forwarding, EXPORT session snapshots, coordinator transport, shared writer consumers, optional-field defaults, release-branch field IDs, and added tests. The implementation uses the appropriate shared abstractions.
APPROVE for the reviewed scope. No builds or tests were run. The supplied current-head snapshot reports successful P0/cloud regressions and FE/BE unit tests, but a failed FE coverage check; required-check rules are unavailable, so this is not a merge-readiness assertion. The existing automated review reports no actionable defects and agrees with this independent assessment.
Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.
Describe the new Parquet output session controls in the English and Chinese current documentation: `enable_parquet_write_page_index` (default `true`) and `parquet_write_data_page_size` (default 1 MiB). Include an OUTFILE example, disabling/restoring the settings, Hive/Iceberg/EXPORT scope, the Hadoop-framed LZ4 page target cap, and tuning tradeoffs. Implementation: apache/doris#69971 Version scope: current/Dev only. These controls are newly introduced on master and are not available in released 4.x or 3.x, so their versioned pages are intentionally unchanged. Validation: both changed pages compile as Docusaurus-compatible MDX; `git diff --check` passes. Changed-link lint reports two existing `parquet_metadata_size_limit` anchor findings already present before this change; the new OUTFILE links resolve. I18n lint flags the intentionally current-only scope described above. A full website build was not run.
…berg WriteInfo (#70209) ### What problem does this PR solve? Issue Number: None Related PR: #69392, #69971 Problem Summary: **In short.** `doris_be_test` does not compile on master since #69971 was merged, so the BE UT of every PR fails with `BE BUILD FAIL`. This fixes the one test case that breaks it. **The problem.** #69392 replaced the `write_path`, `original_write_path` and `target_path` fields of `IPartitionWriterBase::WriteInfo` with one `IcebergOutputLocation location`, and updated the callers it knew about. #69971, merged after it, added `VParquetWriterTest.HiveAndIcebergParquetPageIndexOptions`, which still sets the old fields: ``` be/test/format/transformer/vparquet_writer_test.cpp:380:41: error: no matching constructor for initialization of 'VIcebergPartitionWriter' ``` Each PR built alone, so both were green. TeamCity's BE UT has failed this way for PRs merged with master `0eb5131db62` (for example build 1092024 of #69605). **How this PR fixes it.** The case builds its `WriteInfo` as the other Iceberg case in the same file does, `.location = IcebergOutputLocation(".", ".")`. Nothing else changes.
…/JSON/WAL fixes (#70185) ### What problem does this PR solve? Backport the following master PRs to `branch-4.2`, in master merge order: 1. #69672 — Preserve source values and alias semantics in ES scans. 2. #69971 — Add session controls for Parquet page indexes and data-page size. 3. #70155 — Repair ES source predicate regression cases. 4. #70079 — Fix ORC condition-cache seeks past filtered stripe suffixes. 5. #70154 — Preserve JSON and WAL rows with empty file projections. The ES changes are adapted to the existing 4.2 FE planner and BE source locations. Mapping safety, scan-wide read mode, local residuals, native `esquery` handling and alias routing retain the upstream behavior without introducing the master connector framework. Added adapter tests cover both scan protocols, aliases, missing-value negations and native residual rejection. Preserve existing branch-specific ES regression coverage. Bring in the previously absent predicate suite, adapting its plan assertions to 4.2's `LOCAL_PREDICATES` output and retaining LIMIT result checks. Parquet variables use the 4.2 annotation interface and the original optional Thrift field IDs. The ORC dependency is pinned to the same fixed commit as #70079. The ES implementation also preserves source numeric and JSONB predicate semantics after scan-wide fallback. Numeric-token rounding must not make a remote predicate discard a row matching the value Doris materializes; empty JSONB objects remain nonnull even when ES indexes no leaf. Docvalue eligibility and pushdown are retained. Mapped date formats and their order are carried per concrete backing index through the existing property map to both scan protocols. Calendar/epoch alternatives now distinguish basic_date from epoch_millis, for strings and source numeric tokens. Numeric docvalues and older FE numeric requests retain their epoch decoder. Typed alias selection consistently uses the requested mapping type in schema inference and planning. The pinned ORC decoder's libdeflate dependency is included on ARM, matching master. ### Release note Fix ES source-value filtering, mapped dates and typed alias query correctness, ORC condition-cache seek crashes/missing rows and its ARM link dependency, and JSON/WAL row counts for empty file projections. Add Parquet output page-index and data-page-size session controls. ### Validation - FE compilation and Checkstyle passed. ES validation: 81 tests (48 datasource/planner tests and 33 legacy ES tests), 0 failures/errors, 1 external-environment skip. The 9 Parquet session-variable tests passed. - Rebuilt the pinned ORC dependency under AddressSanitizer: 56 focused tests passed, including 32 filtered-stripe-suffix seek cases. - Compiled all 10 affected Doris C++ implementation/test translation units and the ORC fixture generator. The regenerated fixture is byte-for-byte identical to the committed fixture. - clang-format 16 checks passed for all affected Doris C++ sources and headers. Modified Groovy suites compiled successfully. - The ES reader/parser implementations and test translation units compiled. The shared DATETIMEV2 date-format algorithm passed 21 standalone ASAN checks. CMake dependency checks passed for aarch64, arm64 and x86_64; an actual ARM link was not run locally. - Added `test_es_source_review_cases.groovy` with SQL-to-SQL checks against an internal reference table for numeric/JSONB source predicates, LIMIT, epoch-only formats, and explicit legacy DATETIMEV2 basic_date columns. It covers both scanners and parallel-scroll settings, waits for legacy metadata readiness, and restores session settings. The suite compiled; no golden files were handwritten. - The applicable ES fixes are synchronized to master in #70205. Master already links libdeflate on every architecture. - Full Doris BE unit-test execution and live external SQL regression were not performed locally; CI validation is requested. ### Check List (For Author) - Test - [x] Unit Test - [x] Regression tests included - Behavior changed: - [x] Yes; the upstream behavior is backported to 4.2. - Does this need documentation? - [x] Parquet controls are documented by the upstream documentation PR: apache/doris-website#4581 ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
What problem does this PR solve?
Parquet output relies on implicit page-index and data-page-size defaults, so users cannot explicitly omit indexes or tune the target page size.
Add two validated session variables shared by OUTFILE, EXPORT, Hive and Iceberg Parquet writers:
enable_parquet_write_page_index: write ColumnIndex/OffsetIndex metadata; defaults totrue, preserving the current master behavior.parquet_write_data_page_size: target uncompressed data-page size in bytes; defaults to1048576, accepts positive 32-bit integers, and applies with indexes enabled or disabled.Keep the existing 128 KiB target cap for Hadoop-framed LZ4. Other codecs use the requested target. Row-group size, dictionary-page size, and data-page/format versions are unchanged. Optional query fields preserve existing defaults when omitted by older FEs.
Release note
Add session controls for Parquet page-index output and target data-page size.
Validation
Check List (For Author)
Check List (For Reviewer who merge this PR)