Uh oh!
There was an error while loading. Please reload this page.
[SPARK-58062][SQL] Add variant_strip_nulls expression - #56864
Conversation
variant_strip_nulls expressionvariant_strip_nulls expression
uros-b
left a comment
There was a problem hiding this comment.
cc @marcuslin123 regarding possible duplication (#57318 (review)).
a0732b1 to
0f18407Compare9d51e2d to
91e8463Compare
harshmotw-db
left a comment
There was a problem hiding this comment.
LGTM! I noticed in the tests that all leaf types are small integers (probably always BYTE). Can we add some tests where other data types like strings and wider integers are also mixed in? Thanks
622c9b0 to
b0dc591CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| arguments = """ | ||
| Arguments: | ||
| * v - A variant value to mutate. | ||
| * include_arrays - An optional boolean (default true). Must be a constant. |
There was a problem hiding this comment.
Why default true? PostgreSQL 18 json_strip_nulls defaults to false (https://www.postgresql.org/docs/devel/functions-json.html):
If strip_in_arrays is true (the default is false), null array elements are also stripped.
There was a problem hiding this comment.
Aslo, note possible issues when stripping array elements ends up renumbering the array. After variant_strip_nulls(v) with the current default, any positional path a user holds ($.a[1]) silently points at different data. That's a surprising side effect for a function that reads like it only removes nulls, so it seems better as opt-in?
There was a problem hiding this comment.
Also, strip_in_arrays sounds a bit better than include_arrays here? cc @srielau@harshmotw-db
There was a problem hiding this comment.
variant_strip_nulls mirrors BigQuery's JSON_STRIP_NULLS, where include_arrays defaults to TRUE, so name and default both match BigQuery. PostgreSQL's strip_in_arrays=false is a backward-compat default (arrays were untouched before PG 18), not an analytics preference. BigQuery frames the function as data compaction ( "JSON_STRIP_NULLS compresses the data by removing JSON nulls ... helpful for reducing data size during exports" ) which makes sense fot DW/analytics workload and for that use case you generally want array nulls stripped too.
| val result = super.checkInputDataTypes() | ||
| if (result.isFailure) { | ||
| result | ||
| } else if (!includeArrays.foldable) { |
There was a problem hiding this comment.
The value is passed through StaticInvoke and read per row at runtime, so a non-foldable boolean column would work here with no code changes. Could you just please confirm, is this restriction deliberate?
It's also inconsistent with the closest sibling: VariantSet.createIfMissing is a plain QuaternaryExpression child with no foldability check and accepts dynamic expressions, even though its PySpark docstring claims "Must be a constant." So today the family documents the same constraint in two places and enforces it in one.
There was a problem hiding this comment.
Yes, it is deliberate. It makes sense for a flag to be foldable as it represents the function behaviour mode and in a sense it can be seen as two different functions. Regarding variant_set, restriction is added in #57855.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
uros-b
commented
Aug 11, 2026
LGTM, thank you @bojana-db for resolving all comments. Also, thank you @harshmotw-db for review! |
uros-b
commented
Aug 11, 2026
@bojana-db Please resolve conflicts. |
… null contract Co-authored-by: Isaac
Co-authored-by: Uros Bojanic <221401595+uros-b@users.noreply.github.com>
5fed73b to
6346d6fCompare### What changes were proposed in this pull request?
Adds the SQL function` variant_strip_nulls(v[, includeArrays])`, which recursively removes Variant null fields from objects (and, by default, Variant null elements from arrays) in a Variant value.
Details:
- Object path: a field whose value is a Variant null (the JSON null literal) is removed entirely; recursion walks nested objects and arrays at all levels;
- includeArrays (BOOLEAN, default true): when true, Variant null elements are also removed from arrays; when false, array null elements are kept, but null fields of objects nested anywhere (including inside arrays) are still stripped;
- Empty containers are preserved: an object/array emptied by stripping stays {} / []; the parent is never collapsed to NULL;
- Non-container inputs — scalars and a top-level Variant null (parse_json('null')) — are returned unchanged;
- NULL intolerant: any NULL argument returns NULL.
### Why are the changes needed?
There is no built-in way to drop Variant null entries from a Variant; today users must convert it to another type (e.g. a map) and back.
### Does this PR introduce _any_ user-facing change?
Yes, a new SQL function (and Scala/Python `functions` API.
### How was this patch tested?
Unit tests.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code with Claude Opus 4.8
Closes#56864 from bojana-db/variant-strip-nulls.
Authored-by: bojana-db <bojana.zecevic@databricks.com>
Signed-off-by: Uros Bojanic <221401595+uros-b@users.noreply.github.com>
(cherry picked from commit 2740493)
Signed-off-by: Uros Bojanic <221401595+uros-b@users.noreply.github.com>uros-b
commented
Aug 11, 2026
Adds the SQL function` variant_strip_nulls(v[, includeArrays])`, which recursively removes Variant null fields from objects (and, by default, Variant null elements from arrays) in a Variant value.
Details:
- Object path: a field whose value is a Variant null (the JSON null literal) is removed entirely; recursion walks nested objects and arrays at all levels;
- includeArrays (BOOLEAN, default true): when true, Variant null elements are also removed from arrays; when false, array null elements are kept, but null fields of objects nested anywhere (including inside arrays) are still stripped;
- Empty containers are preserved: an object/array emptied by stripping stays {} / []; the parent is never collapsed to NULL;
- Non-container inputs — scalars and a top-level Variant null (parse_json('null')) — are returned unchanged;
- NULL intolerant: any NULL argument returns NULL.
There is no built-in way to drop Variant null entries from a Variant; today users must convert it to another type (e.g. a map) and back.
Yes, a new SQL function (and Scala/Python `functions` API.
Unit tests.
Generated-by: Claude Code with Claude Opus 4.8
Closesapache#56864 from bojana-db/variant-strip-nulls.
Authored-by: bojana-db <bojana.zecevic@databricks.com>
Signed-off-by: Uros Bojanic <221401595+uros-b@users.noreply.github.com>
What changes were proposed in this pull request?
Adds the SQL function
variant_strip_nulls(v[, includeArrays]), which recursively removes Variant null fields from objects (and, by default, Variant null elements from arrays) in a Variant value.Details:
Why are the changes needed?
There is no built-in way to drop Variant null entries from a Variant; today users must convert it to another type (e.g. a map) and back.
Does this PR introduce any user-facing change?
Yes, a new SQL function (and Scala/Python
functionsAPI.How was this patch tested?
Unit tests.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code with Claude Opus 4.8