feat: Allow null variant scalar - #3287
Conversation
Add stats_schema, which walks a table schema and produces the Adaptive Metadata Tree stats schema (per-column stats structs with field IDs derived from field_id_to_statistics_base). Implemented as a SchemaTransform using the fallible-filtering carrier: primitive/variant columns become stats structs, struct columns recurse, and array/map columns and empty structs are dropped. Co-authored-by: Isaac
Co-authored-by: emkornfield <emkornfield@gmail.com>
`Scalar::to_array` / `append_null` rejected `DataType::Variant` with "Variant is not supported as scalar yet", so a null variant literal could not be materialized. A variant is physically a struct (metadata/value, plus any shredded fields), and `make_builder` already yields a StructBuilder for it, so fold `Variant` into the `Struct` null path: a null variant builds as a null struct. Only the null case is reachable -- there is no non-null `Scalar::Variant`. This unblocks emitting (initially null) variant column bounds in the Delta->AMT stats pivot. Co-authored-by: Isaac <no-reply@databricks.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3287 +/- ##
=======================================
Coverage 90.28% 90.29%
=======================================
Files 251 251
Lines 88575 88572 -3
Branches 88575 88572 -3
=======================================
Hits 79973 79973
+ Misses 5718 5715 -3
Partials 2884 2884 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Benchmark results: ✅ PassSummary: 🚀 0 · ✅ 12 · ☑️ 3 · 🚧 0 · ❌ 0 Per-benchmark results (15 rows)
Legend: 🚀 ≥1.15x faster · ✅ faster or unchanged · ☑️ ≤1.03x slower · 🚧 1.03x-1.15x slower · ❌ ≥1.15x slower |
scovich
left a comment
scovich
left a comment
There was a problem hiding this comment.
I think this looks like a reasonable approach. But I'm a bit unclear on why it's useful to be able to represent NULL variant values but not actual variant values?
| append_nulls_as!(array::Decimal128Builder) | ||
| } | ||
| DataType::Struct(ref stype) => { | ||
| // A variant is physically a struct (`metadata`/`value`, plus any shredded fields), so a |
There was a problem hiding this comment.
But variant has an extension type that the current builder (shared with normal structs) seems to not apply?
There was a problem hiding this comment.
I think the extension type is one level up (i.e. the field containing the struct)? That is I believe StructArray only ever contains its child field types, there needs to be another container for the extension type I think.
we also seem to already drop the type in conversion. It looks like to get an actual VariantArray we would need to depend on parquet-variant-compute crate.
Sorry missed this. It is a useful placeholder for generating null columns via expression. This is useful for stubbing out parts of follow-on code for AMT. |
3 participants
What changes are proposed in this pull request?
We allow Variant null scalar to be constructed as a struct.
How was this change tested?
Added unit test. But I'm a little new to variant so not sure if there is more subtlety that needs to be handled here.