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-28029][SQL][TEST] Port int2.sql #24853

Closed
wants to merge 6 commits into from
Closed

[SPARK-28029][SQL][TEST] Port int2.sql #24853

wants to merge 6 commits into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Jun 12, 2019

What changes were proposed in this pull request?

This PR is to port int2.sql from PostgreSQL regression tests. https://github.com/postgres/postgres/blob/REL_12_BETA2/src/test/regress/sql/int2.sql

The expected results can be found in the link: https://github.com/postgres/postgres/blob/REL_12_BETA2/src/test/regress/expected/int2.out

When porting the test cases, found two PostgreSQL specific features that do not exist in Spark SQL:
SPARK-28023: Trim the string when cast string type to other types
SPARK-28027: Add bitwise shift left/right operators

Also, found a bug:
SPARK-28024: Incorrect value when out of range

Also, found three inconsistent behavior:
SPARK-27923: Invalid input syntax for smallint throws exception at PostgreSQL
SPARK-28028: Cast numeric to integral type need round
SPARK-2659: HiveQL: Division operator should always perform fractional division, for example:

select 1/2;

How was this patch tested?

N/A


-- check rounding when casting from numeric
SELECT x, smallint(x) AS int2_value
FROM (VALUES decimal(-2.5),
Copy link
Member Author

Choose a reason for hiding this comment

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

We do not have numeric type, can we use decimal?

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 plan to use DecimalType.SYSTEM_DEFAULT:

case "numeric" | "decimal" =>
// SPARK-26538: handle numeric without explicit precision and scale.
Some(DecimalType. SYSTEM_DEFAULT)

@SparkQA
Copy link

SparkQA commented Jun 12, 2019

Test build #106425 has finished for PR 24853 at commit 6cf1d6a.

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

@wangyum wangyum changed the title [WIP][SPARK-28029][SQL][TEST] Port int2.sql [SPARK-28029][SQL][TEST] Port int2.sql Jun 14, 2019
@SparkQA
Copy link

SparkQA commented Jun 14, 2019

Test build #106520 has finished for PR 24853 at commit e156649.

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

@SparkQA
Copy link

SparkQA commented Jun 15, 2019

Test build #106535 has finished for PR 24853 at commit b7a62b6.

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

@dongjoon-hyun
Copy link
Member

Retest this please.

-- !query 6 schema
struct<five:string,f1:smallint>
-- !query 6 output
-1234
Copy link
Member

Choose a reason for hiding this comment

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

Ur, is this our test framework behavior? The first column seems to be gone.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This is the behavior if replace '' to '#':
image

SELECT '' AS three, i.* FROM INT2_TBL i WHERE (i.f1 % int('2')) = smallint('0');

-- [SPARK-28024] Incorrect value when out of range
SELECT '' AS five, i.f1, i.f1 * smallint('2') AS x FROM INT2_TBL i;
Copy link
Member

Choose a reason for hiding this comment

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

Let's comment out line 68 since this should throw ERROR: smallint out of range.

SELECT '' AS five, i.f1, i.f1 * int('2') AS x FROM INT2_TBL i;

-- [SPARK-28024] Incorrect value when out of range
SELECT '' AS five, i.f1, i.f1 + smallint('2') AS x FROM INT2_TBL i;
Copy link
Member

Choose a reason for hiding this comment

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

ditto for this due to ERROR: smallint out of range.

SELECT '' AS five, i.f1, i.f1 + int('2') AS x FROM INT2_TBL i;

-- [SPARK-28024] Incorrect value when out of range
SELECT '' AS five, i.f1, i.f1 - smallint('2') AS x FROM INT2_TBL i;
Copy link
Member

Choose a reason for hiding this comment

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

ditto due to ERROR: smallint out of range

SELECT '' AS five, i.f1, i.f1 - int('2') AS x FROM INT2_TBL i;

-- The result is different because [SPARK-2659]
SELECT '' AS five, i.f1, i.f1 / smallint('2') AS x FROM INT2_TBL i;
Copy link
Member

Choose a reason for hiding this comment

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

In case of SPARK-2659, we already made a decision and will not change. So, let's use div' instead of /. I believe this is the same reason why we use smallint('2')instead ofint2 '2'`

Copy link
Member

Choose a reason for hiding this comment

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

The description should be adjusted accordingly like this.

- The result is different because [SPARK-2659]
+ PostgreSQL `/` is the same with Spark `div` since SPARK-2659.

SELECT '' AS five, i.f1, i.f1 / smallint('2') AS x FROM INT2_TBL i;

-- The result is different because [SPARK-2659]
SELECT '' AS five, i.f1, i.f1 / int('2') AS x FROM INT2_TBL i;
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

@SparkQA
Copy link

SparkQA commented Jul 9, 2019

Test build #107391 has finished for PR 24853 at commit a41fb1f.

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

@SparkQA
Copy link

SparkQA commented Jul 9, 2019

Test build #107395 has finished for PR 24853 at commit bc8234a.

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

@SparkQA
Copy link

SparkQA commented Jul 9, 2019

Test build #107390 has finished for PR 24853 at commit b7a62b6.

  • 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 Jul 9, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Jul 9, 2019

Test build #107398 has finished for PR 24853 at commit bc8234a.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @wangyum .
Merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants