tom-eon opened a new pull request, #1036: URL: https://github.com/apache/arrow-go/pull/1036
### Rationale for this change The pre-first-page dictionary cost check (`checkDictionarySizeLimit`, mirroring parquet-mr's `FallbackValuesWriter.shouldFallBack`) compares **uncompressed** sizes: the dictionary is discarded when `dictSize + encodedSize >= rawSize`. With a page compressor configured this can discard dictionaries that clearly win after compression — bit-packed dictionary indices are near-incompressible but small, while PLAIN-encoded values often compress far less than the raw-size comparison assumes. In our benchmarks (ZSTD level 3), a mid-cardinality `Time64` column written PLAIN after the fallback was 2× larger than the same column dictionary-encoded (7.1 MB vs 3.6 MB compressed); mid-cardinality integer columns lost 20%+. Callers that decide dictionary usage from their own measurements (e.g. by trial-encoding a sample of the actual data with the actual codec) currently have no way to make an explicit `WithDictionaryFor(col, true)` stick — the heuristic silently overrides it. **Alternative considered:** making the check itself compression-aware — e.g. trial-compressing both branches of the first data page and keeping the winner, or letting callers supply per-column compressibility estimates for the comparison. That fixes the misprediction for everyone rather than only for callers who opt out, but it costs an extra encode+compress of the first page per column (or a new estimate-supplying API surface), needs a guard band to avoid paying it on clear-cut cases, and still guesses for callers who already measured. The opt-out is minimal, preserves the default exactly, and doesn't preclude a compression-aware check later — if there's interest in that direction I'm happy to discuss it in a follow-up. ### What changes are included in this PR? A new writer property, `parquet.WithDictionaryCostFallback(enabled bool)`, default `true` (existing behavior unchanged). When set to `false`, the pre-first-page cost-based fallback is skipped, so a requested dictionary is kept; the `DictionaryPageSizeLimit` overflow fallback remains in effect regardless of the setting. ### Are these changes tested? Yes — new tests in `parquet/file/dict_cost_fallback_test.go` covering: (1) default behavior still discards a non-paying dictionary, (2) with the option disabled an explicitly requested dictionary is kept, (3) the `DictionaryPageSizeLimit` fallback still applies when the cost fallback is disabled. Existing `parquet/file` and `parquet/internal/encoding` suites pass. ### Are there any user-facing changes? A new opt-in writer property; default behavior is unchanged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
