uros-b commented on code in PR #56864:
URL: https://github.com/apache/spark/pull/56864#discussion_r3735148947
##########
sql/core/src/test/scala/org/apache/spark/sql/VariantSuite.scala:
##########
@@ -680,6 +680,73 @@ class VariantSuite extends SharedSparkSession with
ExpressionEvalHelper {
}
}
+ test("variant_strip_nulls with literal arguments") {
+ def rows(results: Any*): Seq[Row] = results.map(Row(_))
+
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json('{\"a\": 1, \"b\":
null, \"c\": 3}')))"),
+ rows("""{"a":1,"c":3}"""))
+
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json('[1, null, 3]')))"),
+ rows("[1,3]"))
+
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json(" +
+ "'{\"a\": [null, 3, {\"b\": null, \"c\": [null, 1]}], \"d\": null, " +
+ "\"e\": {\"f\": null, \"g\": 2}}')))"),
+ rows("""{"a":[3,{"c":[1]}],"e":{"g":2}}"""))
+
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json(" +
+ "'{\"a\": 100000, \"b\": null, \"c\": 10000000000, \"d\": \"hello
world\"}')))"),
+ rows("""{"a":100000,"c":10000000000,"d":"hello world"}"""))
+
+ // include_arrays = false keeps array null elements but still strips null
fields of objects.
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(" +
+ "parse_json('[{\"a\": 1, \"b\": null}, null, {\"c\": null, \"d\":
4}]'), false))"),
+ rows("""[{"a":1},null,{"d":4}]"""))
+
+ // Empty containers are preserved.
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json('{\"a\": null}')))"),
+ rows("{}"))
+
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json('[null, null]')))"),
+ rows("[]"))
+
+ // Top-level variant null is unchanged.
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(parse_json('null')))"),
+ rows("null"))
+
+ checkAnswer(
+ sql("SELECT to_json(variant_strip_nulls(CAST(NULL AS VARIANT)))"),
+ rows(null))
+ }
+
+ test("variant_strip_nulls with dynamic arguments") {
Review Comment:
Because the flag goes through StaticInvoke rather than being extracted at
planning time, variant_strip_nulls(v, some_boolean_column) is accepted and
evaluated per row. That's arguably a feature, but the test named "with dynamic
arguments" only varies the variant input; the boolean is a literal in both
checkAnswer calls (lines 741 and 746), so the per-row path is never covered.
Either add a case with a boolean column, or decide the flag should be
foldable-only and reject non-literals. Worth being deliberate about, since it's
the kind of thing that gets locked in at release. If literals are the intent,
the test name could say so.
--
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]
