introduce optional rle reads from parquet - #24227
Rich-T-kid wants to merge 9 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24227 +/- ##
==========================================
+ Coverage 81.92% 81.94% +0.01%
==========================================
Files 1134 1134
Lines 425990 426599 +609
Branches 425990 426599 +609
==========================================
+ Hits 348987 349558 +571
- Misses 56304 56332 +28
- Partials 20699 20709 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@adriangb pinging you since you seem interested in parquet related speed ups 👍 |
|
If I understand correctly the goal is to evaluate filters during filter pushdown against dictionary / RLE encoded columns? We can't propagate these dynamic type changes to the rest of the query plan / scan. Is that right? |
no not exactly. The goal of this PR is to keep RLE parquet columns in their compacted form by materializing them as dictionary arrays instead of regular strings.
exactly! This is why it needs to be done as early in the plan as possible. we inspect |
0479f9c to
6e1cbc1
Compare
Why only RLE and not dictionaries as well? How does this compare to / relate to the It also looks like this goes through I'd be more interested in seeing something at the parquet scan level that was able to e.g. optimize how row filters are applied by applying them to the dictionary instead of expanding into |
6a4e89d to
d1273bf
Compare
my bad when I say RLE i'm referring to
I agree, ill update the PR to target all parquet scans. |
|
ideally we surface columns that are physically RLE_DICTIONARY-encoded in the parquet file as Arrow To know whether a specific column is RLE_DICTIONARY-encoded you need to read the parquet file footer. For the Downstream physical operators ( So when the flag is enabled we promote all string/binary columns to dict at planning time, not just the ones that are actually RLE-encoded, because that's the only way to guarantee schema consistency across all parquet scan paths without introducing file I/O into the planning stage. I feel like i'm missing something here. if we could take a peak at the parquets metadata before physical planning and change the schema for all operators from the point forward that would be perfect. Im not sure this is currently possible |
717d31a to
fc360e8
Compare
|
@adriangb When the flag is on, DataFusion promotes string and binary columns that are physically RLE_DICTIONARY encoded in the parquet file to Dictionary(Int32, Utf8) or Dictionary(Int32, Binary) at schema inference time. This applies to all parquet scans regardless of how the table was registered. the PR is ready for review |
|
bc5011f to
6899579
Compare
…ew parquet option
…w in SLT - common_dictionary_key_type: add Int64 arm before catch-all so UInt64+Int64 selects UInt64 (wider) rather than silently narrowing to Int64 - update test expectation from Int64 to UInt64 for that case - SLT high-cardinality test: write rle.parquet via arrow_cast to Dictionary(Int8, Utf8) so the file embeds an Int8-keyed Arrow schema; add a standalone check confirming the round-trip; this exercises the actual Dict(Int8)+plain-130-values overflow path that the previous plain-Utf8 write never triggered
7c9114c to
cb3a578
Compare
@kumarUjjawal just fixed the merge conflicts. Im not sure which benchmark exactly to run 🤔 |
|
run benchmark tpch tpcds clickbench_partitioned wide_schema env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true" |
Started benchmakr. Let's see if it works 🤞 |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark tpch
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark tpcds
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark clickbench_partitioned
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark wide_schema
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
show benchmark queue |
|
Hi @kumarUjjawal, you asked to view the benchmark queue (#24227 (comment)).
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark tpch
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"CPU Details (lscpu)Details
Memory Pool PeaksPeak Base:
Pool accounting vs. process RSS Max pool peak is the largest reservation any single query in the run reached; peak RSS covers the whole invocation, including data loading and allocator retention, and the two high-water marks need not coincide in time. The gap is therefore an upper bound on what the pool did not account for, not a measurement of it.
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark tpcds
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"CPU Details (lscpu)Details
Memory Pool PeaksPeak Base:
Pool accounting vs. process RSS Max pool peak is the largest reservation any single query in the run reached; peak RSS covers the whole invocation, including data loading and allocator retention, and the two high-water marks need not coincide in time. The gap is therefore an upper bound on what the pool did not account for, not a measurement of it.
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing cb3a578 (cb3a578) to cb3a578 diff Run configurationrun benchmark clickbench_partitioned
env:
DATAFUSION_RUNTIME_MEMORY_LIMIT: "32G"
baseline:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "false"
changed:
ref: "cb3a57806e64fa8d95a5ec2e226d34ba5a97af3f"
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"CPU Details (lscpu)Details
Memory Pool PeaksPeak Base:
Pool accounting vs. process RSS Max pool peak is the largest reservation any single query in the run reached; peak RSS covers the whole invocation, including data loading and allocator retention, and the two high-water marks need not coincide in time. The gap is therefore an upper bound on what the pool did not account for, not a measurement of it.
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: Runner log (last 40 lines)Kubernetes messageFile an issue against this benchmark runner |
|
The new paired benchmark is useful because it compares the same A concrete hypothesis, not yet a proven attribution: For #24117 I am keeping the staged optimizer opt-in and fail-closed: exact NDV and row-count, physical page evidence, and a high repetition threshold are required before selectively requesting Dictionary on a direct group key. #25185 may help reused dictionary-value hashing, but its standalone benchmark is not yet end-to-end evidence that it offsets this regression. |
|
@kumarUjjawal any status update on this? |
Based on the reported benchmark number:
These results reproduce the earlier performance direction. I think the implementation needs to avoid automatically promoting high-cardinality or dictionary-fallback columns. |
@kumarUjjawal That is expected with this implementation. The larger epic #24111, specifically #24117, will handle deciding on a per-query basis whether to read the column in as Dictionary or as Utf8/Utf8View. The point of this feature flag is to let users enable direct RLE_DICTIONARY reads from Parquet files in DataFusion, for cases where they know the shape of their data and query well enough to make that decision themselves. By default, this flag will be turned off. This line of thinking is in line with what alamb mentioned here #24111 (comment) |
|
Thank you @Rich-T-kid I will give another pass. |
|
Do we have any query where this option makes the query faster? |
@alamb theres 6 queries that tpcds show sped up but these are more than likely within noise. I think theres still a decent chunk of work left to do in aggregations/filter operators to make dictionary arrays faster. |
|
Thank you @Rich-T-kid for moving this forward. Since this is a larger change I will need second pair of eyes on this before this can be approved. Let's see if anyone has time to take a look. Can you share the work in discord that might bring some people over. |
a large portion of this PR is test!
Which issue does this PR close?
Rationale for this change
When DataFusion reads a parquet file with dictionary-encoded string or binary columns, it currently decodes the dictionary and returns plain Utf8/Binary arrays, discarding the encoding. For low-cardinality columns (status, country, category, etc.) this doesn't take full advantage of the compacted format parquet gives the engine Preserving the dictionary encoding reduces memory usage and can improve aggregation performance on these columns.
What changes are included in this PR?
Adds
datafusion.execution.parquet.enable_rle_to_dictionarywith a default offalse.When enabled for inferred-schema Parquet tables, DataFusion inspects Parquet footer metadata and promotes top-level string/binary columns with dictionary pages to Arrow dictionary types. Mixed dictionary/plain files are normalized before schema merge when the value types are compatible. At scan time, the parquet opener passes the promoted schema to arrow-rs so those columns can be read as dictionary arrays directly.
Tables with a user-supplied schema are not promoted because DataFusion does not use footer metadata to infer their schema.
Are these changes tested?
yes.
datafusion/sqllogictest/test_files/parquet_rle_to_dictionary.sltdatafusion/datasource-parquet/src/schema_coercion.rsuniform_dict_schemas_respects_value_type_familieschecks that mixed file schemas are normalized only across compatible string/binary families.rle_schema_coercion_respects_dictionary_value_typechecks scan-time coercion into dictionary types, including incompatible cases and the flag-off path.datafusion/datasource-parquet/src/opener/mod.rstest_rle_binary_column_promotionverifies the opener passes a promotedDictionary(Int32, Binary)schema to arrow-rs so binary columns can be read as dictionary arrays directly.Are there any user-facing changes?
New session config option:
SET datafusion.execution.parquet.enable_rle_to_dictionary = true. Default is false so existing behavior is unchanged.