Uh oh!
There was an error while loading. Please reload this page.
fix: field stats parquet aggregation - #1675
Conversation
calculate field stats directly from parquet record batches remove old datafusion table path logic add bounded high-cardinality handling per-file stats IDs for correct flattened-row aggregation improve logging and keep partial stats when individual parquet batches fail
WalkthroughThis PR replaces SQL-based per-field statistics with a single-pass Arrow/Parquet reader pipeline. It introduces bounded distinct value tracking using HyperLogLog approximation, semaphore-controlled concurrency, and updates statistics query generation with new aggregation CTEs. ChangesField Statistics Computation Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/storage/field_stats.rs (1)
929-929:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSQL injection vulnerability: incorrect escaping for string literal context.
The
dataset_nameis escaped for double-quoted identifiers ("→""), but it's used in a single-quoted string literal at line 952. Ifdataset_namecontains a single quote (e.g.,test'--), it will break out of the SQL string and allow injection.🔒 Proposed fix
- let dataset_name = dataset_name.replace('"', "\"\"");+ let dataset_name = dataset_name.replace('\'', "''");
Uh oh!
There was an error while loading. Please reload this page.
calculate field stats directly from parquet record batches remove old datafusion table path logic add bounded high-cardinality handling per-file stats IDs for correct flattened-row aggregation improve logging and keep partial stats when individual parquet batches fail
calculate field stats directly from parquet record batches remove old datafusion table path logic
add bounded high-cardinality handling
per-file stats IDs for correct flattened-row aggregation
improve logging and keep partial stats when individual parquet batches fail
Summary by CodeRabbit
Refactor
Tests