[SPARK-49443][SQL][PYTHON] Implement to_variant_object expression and make schema_of_variant expressions print OBJECT for for Variant Objects #47907

harshmotw-db · 2024-08-28T09:10:29Z

What changes were proposed in this pull request?

This PR prohibits casts from data types containing structs or maps to variant and introduces a new expression to_variant_object which allows converting nested types to variants and retains the old functionality. This PR also changes the behavior of the schema_of_variant and schema_of_variant_agg expressions where they now print OBJECT instead of STRUCT (which is not technically correct).

Why are the changes needed?

Cast from structs to variant objects should not be legal since variant objects are unordered bags of key-value pairs while structs are ordered sets of elements of fixed types. Therefore, casts between structs and variant objects do not behave like casts between structs. Example (produced by Serge Rielau):

scala> spark.sql("SELECT cast(named_struct('c', 1, 'b', '2') as struct<b int, c int>)").show()
+------------------------+
|named_struct(c, 1, b, 2)|

+------------------------+
|{1, 2}|

+------------------------+

Passing a struct into VARIANT loses the position
scala> spark.sql("SELECT cast(named_struct('c', 1, 'b', '2')::variant as struct<b int, c int>)").show()
+-----------------------------------------+
|CAST(named_struct(c, 1, b, 2) AS VARIANT)|

+-----------------------------------------+
|{2, 1}|

+-----------------------------------------+

Casts from maps to variant objects should also not be legal since they represent completely orthogonal data types. Maps can represent a variable number of key value pairs based on just a key and value type in the schema but in objects, the schema (produced by schema_of_variant expressions) will have a type corresponding to each value in the object. Objects can have values of different types while maps cannot and objects can only have string keys while maps can also have complex keys.

We should therefore prohibit the existing behavior of allowing explicit casts from structs and maps to variants as the variant spec currently only supports an object type which is remotely compatible with structs and maps. We introduce a new expression that converts schemas containing structs and maps to variants (where these types are converted to objects). We will call it to_variant_object.

Does this PR introduce any user-facing change?

Yes, it introduces the to_variant_object expression and changes the behavior of the schema_of_variant/schema_of_variant_agg expressions.

How was this patch tested?

Several unit tests with codegen enabled/disabled.

Was this patch authored or co-authored using generative AI tooling?

Yes.
Generated-by: GitHub Copilot, perplexity.ai

harshmotw-db · 2024-08-28T09:41:27Z

I haven't generated the golden files yet as I don't remember which commands to run. I'll figure it out based on test failures.

gene-db

@harshmotw-db Thanks for this functionality! I left a few comments.

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala

python/pyspark/sql/tests/test_functions.py

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala

cloud-fan · 2024-08-29T14:06:25Z

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala


+// scalastyle:off line.size.limit
+@ExpressionDescription(
+  usage = "_FUNC_(expr) - Convert a nested input (array/map/struct) into a variant where maps and structs are converted to variant objects which are unordered unlike SQL structs. Input maps can only have string keys.",


How do we define the element order in the resulting variant object? Random?

Yes and no. From a logical perspective, the keys should be thought of as random and the users should not assume anything about the order.
However, in the spec, the field IDs are sorted based on the lexicographic order of the keys. This is to make it possible to binary search for the required key.

harshmotw-db · 2024-08-29T19:58:46Z

Note to reviewers: There is currently a bug when using UTF8_LCASE collation. I am looking into it.

scala> sql("""select to_variant_object(map("a" collate utf8_lcase, 2))""").collect()
org.apache.spark.SparkException: [INTERNAL_ERROR] The Spark SQL phase optimization failed with an internal error. You hit a bug in Spark or the Spark plugins you use. Please, report this bug to the corresponding communities or vendors, and provide the full stack trace. SQLSTATE: XX000

harshmotw-db · 2024-08-29T20:39:43Z

Note to reviewers: There is currently a bug when using UTF8_LCASE collation. I am looking into it.

scala> sql("""select to_variant_object(map("a" collate utf8_lcase, 2))""").collect()
org.apache.spark.SparkException: [INTERNAL_ERROR] The Spark SQL phase optimization failed with an internal error. You hit a bug in Spark or the Spark plugins you use. Please, report this bug to the corresponding communities or vendors, and provide the full stack trace. SQLSTATE: XX000

Thia issue has been fixed

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala

harshmotw-db · 2024-09-03T02:23:53Z

@HyukjinKwon Can you go over the Python changes in this PR?

python/pyspark/sql/functions/builtin.py

gene-db

@harshmotw-db Thanks! I left a few minor comments.

LGTM

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala

harshmotw-db · 2024-09-06T15:19:55Z

@cloud-fan Can you go over this PR again whenever you're available?

…o_variant_object

harshmotw-db · 2024-09-09T18:24:52Z

@cloud-fan There may be an issue with the Scala linter test. It was passing earlier and is failing after a very minor commit. It says there are lint failures in the sql/connect and connector/connect spaces which this PR is not even modifying. It recommends a command to fix these issues but that command is making several unrelated changes across the codebase.

cloud-fan · 2024-09-10T14:09:25Z

sql/api/src/main/scala/org/apache/spark/sql/functions.scala

+  /**
+   * Converts a column containing nested inputs (array/map/struct) into a variants where maps and
+   * structs are converted to variant objects which are unordered unlike SQL structs. Input maps can
+   * only have string keys.


if the input has no array/map/struct, this function is noop?

nvm, the type check will fail for it.

cloud-fan · 2024-09-10T14:17:12Z

The link failure is unrelated, thanks, merging to master!

MaxGekk

@cloud-fan Here is the fix for formatting: #48060

harshmotw-db added 2 commits August 28, 2024 00:56

Fully implemented to_variant_object

7d817d2

fixed schema_of_variant expressions

0f56cc9

github-actions bot added SQL DOCS PYTHON CONNECT labels Aug 28, 2024

fix

1e585a0

fix

015c429

gene-db reviewed Aug 29, 2024

View reviewed changes

cloud-fan reviewed Aug 29, 2024

View reviewed changes

Addressed Gene's comments.

0d8dc2b

harshmotw-db requested review from cloud-fan and gene-db August 29, 2024 16:45

harshmotw-db added 3 commits August 29, 2024 10:38

Update sql-expression-schema.md

7b93564

test fixes

692b4fa

Update test_functions.py

4bea027

fixed collation issue

6cd2644

harshmotw-db added 4 commits August 29, 2024 14:23

fix

5c6da07

Update builtin.py

aedb70c

Update builtin.py

a0bc9cc

Update builtin.py

591c234

chenhao-db approved these changes Aug 30, 2024

View reviewed changes

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala Outdated Show resolved Hide resolved

fixed Chenhao's comment

3d35da5

HyukjinKwon reviewed Sep 3, 2024

View reviewed changes

python/pyspark/sql/functions/builtin.py Outdated Show resolved Hide resolved

HyukjinKwon reviewed Sep 3, 2024

View reviewed changes

python/pyspark/sql/functions/builtin.py Outdated Show resolved Hide resolved

HyukjinKwon reviewed Sep 3, 2024

View reviewed changes

python/pyspark/sql/functions/builtin.py Outdated Show resolved Hide resolved

HyukjinKwon reviewed Sep 3, 2024

View reviewed changes

python/pyspark/sql/functions/builtin.py Show resolved Hide resolved

HyukjinKwon approved these changes Sep 3, 2024

View reviewed changes

Addressed Hyukjin's comments

8093084

gene-db approved these changes Sep 4, 2024

View reviewed changes

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala Show resolved Hide resolved

...st/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala Show resolved Hide resolved

harshmotw-db added 2 commits September 6, 2024 20:14

added minor test

de045d2

Merge branch 'master' of https://github.com/harshmotw-db/spark into t…

1eb3c20

…o_variant_object

cloud-fan reviewed Sep 10, 2024

View reviewed changes

cloud-fan approved these changes Sep 10, 2024

View reviewed changes

cloud-fan closed this in 3709c2e Sep 10, 2024

MaxGekk reviewed Sep 10, 2024

View reviewed changes

[SPARK-49443][SQL][PYTHON] Implement to_variant_object expression and make schema_of_variant expressions print OBJECT for for Variant Objects #47907

[SPARK-49443][SQL][PYTHON] Implement to_variant_object expression and make schema_of_variant expressions print OBJECT for for Variant Objects #47907

Uh oh!

Conversation

harshmotw-db commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

harshmotw-db commented Aug 28, 2024

Uh oh!

gene-db left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cloud-fan Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

harshmotw-db Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

harshmotw-db commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harshmotw-db commented Aug 29, 2024

Uh oh!

Uh oh!

harshmotw-db commented Sep 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gene-db left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

harshmotw-db commented Sep 6, 2024

Uh oh!

harshmotw-db commented Sep 9, 2024

Uh oh!

cloud-fan Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

cloud-fan Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Sep 10, 2024

Uh oh!

MaxGekk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

harshmotw-db commented Aug 28, 2024 •

edited

Loading

harshmotw-db commented Aug 29, 2024 •

edited

Loading