[SPARK-47569][SQL] Disallow comparing variant. #45726
Conversation
| @@ -192,4 +192,22 @@ class VariantSuite extends QueryTest with SharedSparkSession { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| test("group/order/join variant are disabled") { | |||
There was a problem hiding this comment.
Thanks for the tests!
What about sort by? or window partition by, or window order by? Some examples:
select parse_json('') sort by 1
with t as (select 1 as a, parse_json('') as v)
select rank() over (partition by v order by a)
with t as (select 1 as a, parse_json('') as v)
select rank() over (partition by a order by v)
There was a problem hiding this comment.
This is a very good point. Actually, the second statement (partition by v) will not fail as expected, and it is really an error in the type check for Window. Both partitionSpec and orderSpec must be orderable, but the current type check only includes orderSpec. If the partitionSpec contains a map type, the query will fail later in an optimizer rule with INTERNAL_ERROR. I will create another PR to fix the type check for Window.
There was a problem hiding this comment.
If we can merge #45730 before this PR, we can add the second statement to the tests.
|
@cloud-fan Could you help review this PR? Thanks! |
|
thanks, merging to master! |
### What changes were proposed in this pull request? It adds type-checking rules to disallow comparing variant values (including group by a variant column). We may support comparing variant values in the future, but since we don't have a proper comparison implementation at this point, they should be disallowed on the user surface. ### How was this patch tested? Unit tests. Closes apache#45726 from chenhao-db/SPARK-47569. Authored-by: Chenhao Li <chenhao.li@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
What changes were proposed in this pull request?
It adds type-checking rules to disallow comparing variant values (including group by a variant column). We may support comparing variant values in the future, but since we don't have a proper comparison implementation at this point, they should be disallowed on the user surface.
How was this patch tested?
Unit tests.