Skip to content

[59_maintenance] Parquet: list the dictionary page first in page_encoding_stats - #11699

Merged
alamb merged 1 commit into
apache:59_maintenancefrom
Jefffrey:backport_11682
Oct 19, 2026
Merged

alamb merged 1 commit into
apache:59_maintenancefrom
Jefffrey:backport_11682

Conversation

@Jefffrey

Copy link
Copy Markdown
Contributor

…e#11682)

- Closes apache#11659

Since 59.0.0, `ArrowWriter` writes the dictionary page's
`encoding_stats` entry after the data page entries. The file layout did
not change: the dictionary page is still written first. Arrow C++
([`IsColumnChunkFullyDictionaryEncoded`](https://github.com/apache/arrow/blob/9f0341314ca96eb58081d4132c77b460d880b2b0/cpp/src/parquet/file_reader.cc#L63-L88))
requires `encoding_stats[0]` to be the dictionary page. With the 59.x
order, Arrow C++ does not see any parquet-rs column chunk as fully
dictionary encoded, so it cannot use its dictionary fast path.

| | `encoding_stats` (file order) |
|---|---|
| before this PR | `DATA_PAGE RLE_DICTIONARY`, `DICTIONARY_PAGE PLAIN` |
| after this PR (and 58.x) | `DICTIONARY_PAGE PLAIN`, `DATA_PAGE
RLE_DICTIONARY` |

The new order was not a design goal of
apache#10020. It was a side effect: the
deferred-ordering page writer gets the dictionary page last, and the
stats were recorded in the order the pages were produced. Review
accepted this because the spec defines the list as a set
(apache#10051 was closed for that
reason). But a known reader depends on the order, and restoring it
changes only metadata, so the change has no memory or throughput cost.

`GenericColumnWriter::write_dictionary_page` now inserts the dictionary
entry at the front of `encoding_stats` instead of appending it. The
dictionary page is always laid out first, on both the deferred (Arrow)
path and the `SerializedFileWriter` path, so the stats now follow file
order. On the non-deferred path the list is still empty when the
dictionary is written, so its output does not change. When the writer
falls back from dictionary to plain encoding, the result is `DICTIONARY,
RLE_DICTIONARY data, PLAIN data`, which also matches the file layout.

Yes. The new `dictionary_page_encoding_stats_lists_dictionary_first`
test (adapted from apache#10051) checks
that `stats[0]` is the dictionary page. It fails on `main` and passes
with this change. The existing column writer and arrow writer tests
pass.

Only the order of `page_encoding_stats` in files written by
`ArrowWriter`. It goes back to the order used before 59.0.0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the parquet Changes to the parquet crate label Oct 19, 2026
@Jefffrey
Jefffrey marked this pull request as ready for review October 19, 2026 07:43

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified this is a clean backport. Thank you @Jefffrey

@alamb
alamb merged commit dc6cf47 into apache:59_maintenance Oct 19, 2026
17 checks passed
@Jefffrey
Jefffrey deleted the backport_11682 branch October 21, 2026 10:56
@Jefffrey Jefffrey added the bug label Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants