[SPARK-28885][SQL][FOLLOW-UP] Re-enable the ported PgSQL regression tests of SQLQueryTestSuite by maropu · Pull Request #26492 · apache/spark

maropu · 2019-11-13T05:46:03Z

What changes were proposed in this pull request?

SPARK-28885(#26107) has supported the ANSI store assignment rules and stopped running some ported PgSQL regression tests that violate the rules. To re-activate these tests, this pr is to modify them for passing tests with the rules.

Why are the changes needed?

To make the test coverage better.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

SparkQA · 2019-11-13T08:05:01Z

Test build #113669 has finished for PR 26492 at commit bab3711.

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

maropu · 2019-11-13T08:11:44Z

retest this please

maropu · 2019-11-13T10:01:58Z

Thanks for your quick check, @MaxGekk ! cc: @dongjoon-hyun @gengliangwang

SparkQA · 2019-11-13T12:28:40Z

Test build #113682 has finished for PR 26492 at commit bab3711.

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

maropu · 2019-11-13T12:33:36Z

+0 seconds
+	3 hours 4 minutes 5 seconds
+	41393 hours 19 minutes 20 seconds
+	953 hours 32 minutes 1 seconds


This diff is related to the recent change: #26401

Thank you for updating, @maropu .

dongjoon-hyun · 2019-11-13T19:05:08Z

-INSERT INTO fract_only VALUES (5, '0.99994');
+-- INSERT INTO fract_only VALUES (3, '1.0' as decimal(4,4));	-- should fail
+INSERT INTO fract_only VALUES (4, cast('-0.9999' as decimal(4,4)));
+INSERT INTO fract_only VALUES (5, cast('0.99994' as decimal(4,4)));


So, this is converting CASTING DURING INSERTING into CASTING WITH CAST syntax.

Ur... my fault.. I'll update later.

Oh, it was just a comment to clarify. I think it's okay for now AS-IS since we don't support the original query in any way.

But, the original queries throw unsupported exceptions now, so most of queries in this file become invalid in Spark. In the last commit, I changed these string literals into floating-point literals for depending on INSERT assignment casts. Which do you think is better, as-is or the last commit?

dongjoon-hyun · 2019-11-13T19:07:08Z

-INSERT INTO TIMESTAMP_TBL VALUES ('now');
+-- PostgreSQL implicitly casts string literals to data with timestamp types, but
+-- Spark does not support that kind of implicit casts.
+INSERT INTO TIMESTAMP_TBL VALUES timestamp(('now'));


It seems that you wanted the following.
- INSERT INTO TIMESTAMP_TBL VALUES timestamp(('now')); + INSERT INTO TIMESTAMP_TBL VALUES timestamp('now');
Or,
INSERT INTO TIMESTAMP_TBL VALUES (timestamp('now'));

SparkQA · 2019-11-14T04:50:13Z

Test build #113751 has finished for PR 26492 at commit a4636ec.

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

gengliangwang · 2019-11-14T04:55:54Z

@maropu Thanks for reenabling the test cases!

dongjoon-hyun · 2019-11-14T07:05:22Z

ThriftServerQueryTestSuite reuses the queries. It seems that we need to regenerate them too, @maropu . (Also, cc @wangyum ).

dongjoon-hyun · 2019-11-14T07:06:45Z

 -- PostgreSQL implicitly casts string literals to data with timestamp types, but
 -- Spark does not support that kind of implicit casts.
-INSERT INTO TIMESTAMP_TBL VALUES timestamp(('now'));
+INSERT INTO TIMESTAMP_TBL VALUES (timestamp('now'));


Did we regenerate timestamp.sql.out?

Oh... I got it.

dongjoon-hyun · 2019-11-14T07:07:29Z


 -- !query 1
-INSERT INTO TIMESTAMP_TBL VALUES ('now')
+INSERT INTO TIMESTAMP_TBL VALUES timestamp(('now'))


I mean this one~

SparkQA · 2019-11-14T08:05:01Z

Test build #113759 has finished for PR 26492 at commit 202988b.

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

gengliangwang · 2019-11-14T08:08:35Z

retest this please.

SparkQA · 2019-11-14T11:36:01Z

Test build #113766 has finished for PR 26492 at commit 202988b.

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

maropu · 2019-11-15T00:17:26Z

retest this please

SparkQA · 2019-11-15T01:46:21Z

Test build #113823 has finished for PR 26492 at commit 202988b.

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

maropu · 2019-11-20T02:38:07Z

retest this please

SparkQA · 2019-11-20T05:37:46Z

Test build #114124 has finished for PR 26492 at commit 202988b.

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

maropu · 2019-11-20T05:54:12Z

Ur, it seems I need to update the golden file again..

SparkQA · 2019-11-20T13:10:00Z

Test build #114154 has finished for PR 26492 at commit 7c06f0d.

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

dongjoon-hyun

+1, LGTM. Merged to master.

maropu · 2019-11-21T00:10:46Z

Thanks for taking care of it, guys! @dongjoon-hyun @gengliangwang @MaxGekk

MaxGekk approved these changes Nov 13, 2019
View reviewed changes

maropu commented Nov 13, 2019
View reviewed changes

dongjoon-hyun added the SQL label Nov 13, 2019

dongjoon-hyun reviewed Nov 13, 2019
View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-28885][SQL][FOLLOW-UP] Remove the ported PgSQL regression tests from the blacklist of SQLQueryTestSuite~~ [SPARK-28885][SQL][FOLLOW-UP] Re-enable the ported PgSQL regression tests of SQLQueryTestSuite Nov 13, 2019

dongjoon-hyun reviewed Nov 14, 2019
View reviewed changes

maropu added 4 commits November 20, 2019 16:30

Fix
b4de386

Fix
de55659

Fix
52e3503

Update the golden file
7c06f0d

maropu force-pushed the SPARK-28885-FOLLOWUP branch from 202988b to 7c06f0dCompare November 20, 2019 08:50

dongjoon-hyun approved these changes Nov 20, 2019
View reviewed changes

dongjoon-hyun closed this in 6eeb131Nov 20, 2019

Uh oh!

Conversation

maropu commented Nov 13, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

maropu commented Nov 13, 2019

Uh oh!

maropu commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyunNov 13, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 14, 2019

Uh oh!

gengliangwang commented Nov 14, 2019

Uh oh!

dongjoon-hyun commented Nov 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 14, 2019

Uh oh!

gengliangwang commented Nov 14, 2019

Uh oh!

SparkQA commented Nov 14, 2019

Uh oh!

maropu commented Nov 15, 2019

Uh oh!

SparkQA commented Nov 15, 2019

Uh oh!

maropu commented Nov 20, 2019

Uh oh!

SparkQA commented Nov 20, 2019

Uh oh!

maropu commented Nov 20, 2019

Uh oh!

SparkQA commented Nov 20, 2019

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

maropu commented Nov 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dongjoon-hyunNov 13, 2019•
edited
Loading