[SPARK-47569][SQL] Disallow comparing variant.#45726
[SPARK-47569][SQL] Disallow comparing variant.#45726chenhao-db wants to merge 5 commits intoapache:masterfrom
Conversation
gene-db
left a comment
There was a problem hiding this comment.
@chenhao-db Thanks! I left a question.
| } | ||
| } | ||
|
|
||
| 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.
gene-db
left a comment
There was a problem hiding this comment.
@chenhao-db Thanks for fixing and the test cases!
LGTM
|
@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.