nssalian opened a new pull request, #17424: URL: https://github.com/apache/iceberg/pull/17424
## Rationale for the change The current majority-based algorithm introduced in [#14297](https://github.com/apache/iceberg/pull/14297) shreds mixed-type fields to the majority type. A field seen as 95% int + 5% string still emits an int-typed column that misses on the 5% string rows, which fall back to the residual `value`. That `typed_value` column pays its storage and I/O cost while covering only a fraction of the field's rows, and readers must always check both columns. This change requires type uniformity: a field is shredded only when all its observations fall into a single type family after numeric widening. Mixed-type fields stay in the residual `value`; no `typed_value` column is emitted for them. ### Benefits - Reader I/O: no mostly-empty typed_value columns to open, decode, and skip. Wide-schema files get fewer column chunks and less Parquet metadata. - Predicate pushdown and row-group skipping: when a typed_value column exists, its min/max stats cover the entire field, so filter pushdown and stats-based row-group skipping are clean. Under the majority algorithm, minority-type rows in `value` broke stat coverage and forced fallback scans. - Predictable rule: uniform -> shredded, otherwise -> residual. No surprising tie-break outcomes (e.g., 50/50 int-vs-string silently picking STRING via priority table). Clean single-type workloads are unchanged. Benchmark evidence (per-column scoring plus reader wall-clock across 11 workloads at 100k rows) shows uniformity ties or beats the majority algorithm on every workload and wins uniquely on mixed-type inputs (60/40, 95/5, 99/1 int/string, four-way polymorphic). Full numbers in the design document. Design document with benchmarks: [Google Doc](https://docs.google.com/document/d/1OaB45mg9JJ_JRg-ipuvyh4AtOmaGmdWMsy_mJKiu5xs/edit?usp=sharing) ## Changes - `VariantShreddingAnalyzer`: add a uniformity check to `admittedType()`. Returns null when observations span more than one type family after widening; null propagates through existing handling in `analyzeAndCreateSchema`, `buildFieldGroup`, and `createArrayTypedValue`, so rejected fields fall out of the emitted schema and stay in residual `value`. - Removed the now-unreachable `TIE_BREAK_PRIORITY` map; simplified type selection to `families.iterator().next()`. - Rename `getMostCommonType` -> `admittedType` (plus `mostCommonCached`/`Computed` and local renames). Change `combinedCounts` from `Map<PhysicalType, Integer>` to `Set<PhysicalType> families` since counts are no longer needed post-uniformity. - Updated the class-level javadoc. ## Follow up - Will open follow-up issues for each implementation; the iceberg-go change I'll implement myself. ## Tests - `TestVariantShreddingAnalyzer`: 5 new tests covering mixed-primitive rejection, integer widening admission, int+decimal cross-family rejection, mixed object+primitive rejection, and array-element mixing rejection. One existing test updated to use uniform-type observations. - `TestVariantShredding` (Spark v4.0 and v4.1): 3 tests renamed and updated to assert non-shredding on mixed inputs (`testInconsistentType`, `testPrimitiveDecimalType`, `testMixedTypeTieBreaking`). - `TestFlinkVariantShreddingType` (Flink v2.1): 4 tests renamed and updated similarly. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
