Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-22762][TEST] Basic tests for IfCoercion and CaseWhenCoercion #19949

Closed
wants to merge 2 commits into from
Closed

[SPARK-22762][TEST] Basic tests for IfCoercion and CaseWhenCoercion #19949

wants to merge 2 commits into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Dec 12, 2017

What changes were proposed in this pull request?

Basic tests for IfCoercion and CaseWhenCoercion

How was this patch tested?

N/A

@HyukjinKwon
Copy link
Member

Out of curiosity, does anyone know if we are okay with elapsed time now?

@wangyum
Copy link
Member Author

wangyum commented Dec 12, 2017

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Dec 12, 2017

To be clear, I am not against this change. I am asking this because simply I remember I was working on another stuff related with this test but the elapsed time concern was raised before.

I see a bunch of tests being added here and I think it's a valid concern. I simply just wonder the elapsed time (wouldn't be difficult to check) and roughly how many tests you plan to add.

@SparkQA
Copy link

SparkQA commented Dec 12, 2017

Test build #84757 has finished for PR 19949 at commit f9da910.

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

@HyukjinKwon
Copy link
Member

Ah, I just checked the log and seems quite short. I take my words back. I am fine.


CREATE TEMPORARY VIEW t AS SELECT 1;

SELECT CASE WHEN true THEN cast(1 as tinyint) ELSE cast(2 as tinyint) END FROM t;
Copy link
Member

Choose a reason for hiding this comment

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

Just want to confirm whether these queries you added can be executed in Hive?

Copy link
Member Author

Choose a reason for hiding this comment

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

@gatorsmile Two questions:

  1. Hive doesn't have the short type, so can we remove the short type here?
  2. Hive can't execute CREATE TEMPORARY VIEW ..., but can executor CREATE TEMPORARY TABLE ..., Do we add this feature to spark?

Copy link
Member

Choose a reason for hiding this comment

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

In the future, we might do it for supporting the actual TEMP table.

Yeah, please get rid of short.

@gatorsmile
Copy link
Member

LGTM except the comment about short

@SparkQA
Copy link

SparkQA commented Dec 15, 2017

Test build #84942 has finished for PR 19949 at commit 8d04720.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@wangyum
Copy link
Member Author

wangyum commented Dec 15, 2017

retest this please

@SparkQA
Copy link

SparkQA commented Dec 15, 2017

Test build #84948 has finished for PR 19949 at commit 8d04720.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member

Thanks! Merged to master.

@asfgit asfgit closed this in 4677623 Dec 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants