Skip to content

[SPARK-24916][SQL] Fix type coercion for IN expression with subquery - #21871

Closed
wangyum wants to merge 7 commits into
apache:masterfrom
wangyum:SPARK-24916
Closed

[SPARK-24916][SQL] Fix type coercion for IN expression with subquery#21871
wangyum wants to merge 7 commits into
apache:masterfrom
wangyum:SPARK-24916

Conversation

@wangyum

@wangyumwangyum commented Jul 25, 2018

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

The below SQL will throw AnalysisException. but it can success on Spark 2.1.x. This pr fix this issue.

CREATE TEMPORARY VIEW t4 ASSELECT*FROMVALUES
(CAST(1AS DOUBLE), CAST(2AS STRING), CAST(3AS STRING))
AS t1(t4a, t4b, t4c);
CREATE TEMPORARY VIEW t5 ASSELECT*FROMVALUES
(CAST(1ASDECIMAL(18, 0)), CAST(2AS STRING), CAST(3ASBIGINT))
AS t1(t5a, t5b, t5c);
SELECT*FROM t4
WHERE
(t4a, t4b, t4c) IN (SELECT t5a, t5b, t5c FROM t5);

I tested the different combinations of getting commonTypes, it looks like findCommonTypeForBinaryComparison + findWiderTypeWithoutStringPromotionForTwo is the best way.

No.commonTypesdesc
1findCommonTypeForBinaryComparison + findTightestCommonTypeCan't compare double with decimal
2findCommonTypeForBinaryComparison + findWiderTypeForTwoSeems good
3findWiderTypeForTwoCan't compare string with double
4findCommonTypeForBinaryComparison + findWiderTypeWithoutStringPromotionForTwoSame to No.2
5findWiderTypeWithoutStringPromotionForTwoCan't compare string with int
6findCommonTypeForBinaryComparison + findTightestCommonType + findWiderTypeForDecimalSame to No.2

The details can be found in the commit log.

How was this patch tested?

unit tests

@mgaido91

Copy link
Copy Markdown
Contributor

I think this is basically the same of what I proposed in #19635. Unfortunately, that PR got a bit stuck...

SELECT cast('2017-12-12 09:30:00' as date) in (cast('2017-12-12 09:30:00' as date), cast('2017-12-11 09:30:00.0' as timestamp)) FROM t;
SELECT cast('2017-12-12 09:30:00' as date) in (cast('2017-12-12 09:30:00' as date), cast('2017-12-11 09:30:00' as date)) FROM t;

SELECT * FROM t WHERE (cast(1 as tinyint)) IN (SELECT cast(1 as tinyint) FROM t);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to test all the combinations? We need most of such logics should be tested in findWiderTypeWithoutStringPromotionForTwo and we could have just few end to end tests.

@wangyum

Copy link
Copy Markdown
MemberAuthor

Oh. It turns out that @dilipbiswal is talking about that PR. I didn't find it in your recent PR. Let’s wait if the test can pass.

@SparkQA

Copy link
Copy Markdown

Test build #93538 has finished for PR 21871 at commit 8ef142f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@wangyumwangyum closed this Jul 26, 2018
@wangyum

Copy link
Copy Markdown
MemberAuthor

workaround:

SELECT*FROM t4
WHERE
(t4a, t4b, t4c) IN (SELECT t5a, t5b, t5c FROM t5);
->SELECT*FROM t4
WHERE
(t4a, t4b, t4c) IN (SELECT CAST(t5a as DOUBLE), CAST(t5b AS STRING), CAST(t5c AS STRING) FROM t5);

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@wangyum@mgaido91@SparkQA@HyukjinKwon