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-22821][TEST] Basic tests for WidenSetOperationTypes, BooleanEquality, StackCoercion and Division #20006

Closed
wants to merge 3 commits into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Dec 18, 2017

What changes were proposed in this pull request?

Test Coverage for WidenSetOperationTypes, BooleanEquality, StackCoercion and Division, this is a Sub-tasks for SPARK-22722.

How was this patch tested?

N/A

@SparkQA
Copy link

SparkQA commented Dec 18, 2017

Test build #85039 has finished for PR 20006 at commit 7ee9aec.

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

SELECT true = cast('1' as binary) FROM t;
SELECT true = cast('1' as boolean) FROM t;
SELECT true = cast('2017-12-11 09:30:00.0' as timestamp) FROM t;
SELECT true = cast('2017-12-11 09:30:00' as date) 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.

Also please add the cases for false = 0

@SparkQA
Copy link

SparkQA commented Dec 18, 2017

Test build #85052 has finished for PR 20006 at commit d4380bf.

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

SELECT true = cast('1' as binary) FROM t;
SELECT true = cast(1 as boolean) FROM t;
SELECT true = cast('2017-12-11 09:30:00.0' as timestamp) FROM t;
SELECT true = cast('2017-12-11 09:30:00' as date) 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.

Could we just use one side?

For example, let us get rid of true = cast(1 as tinyint) and keep cast(1 as tinyint) = true?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should keep both, we have some below usage:

case EqualTo(bool @ BooleanType(), Literal(value, _: NumericType))
if falseValues.contains(value) => Not(bool)
case EqualTo(Literal(value, _: NumericType), bool @ BooleanType())
if trueValues.contains(value) => bool

case (StringType, TimestampType) => Some(StringType)
case (TimestampType, StringType) => Some(StringType)

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

SELECT cast(1 as string) / cast(1 as tinyint) 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.

Duplicates?


CREATE TEMPORARY VIEW t AS SELECT 1;

SELECT stack(cast(1 as tinyint), cast(1 as tinyint), null) 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.

This suite is not needed.

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

-- EXCEPT
Copy link
Member

Choose a reason for hiding this comment

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

We do not need EXCEPT and INTERSECT

@SparkQA
Copy link

SparkQA commented Dec 19, 2017

Test build #85103 has finished for PR 20006 at commit cce792d.

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

@gatorsmile
Copy link
Member

LGTM

@asfgit asfgit closed this in 6129ffa Dec 19, 2017
@gatorsmile
Copy link
Member

Thanks! Merged to master.

Sign up for free to 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.

3 participants