[SPARK-28617][SQL][TEST] Fix misplacement when comment is at the end of the query by wangyum · Pull Request #25357 · apache/spark

wangyum · 2019-08-05T03:26:52Z

What changes were proposed in this pull request?

This PR fixes the issue of misplacement when the comment at the end of the query. Example:
Comment for SELECT date '5874898-01-01':

spark/sql/core/src/test/resources/sql-tests/inputs/pgSQL/date.sql

Line 200 in 2d74f14

SELECT date '5874898-01-01'; -- out of range

But the golden file is:

spark/sql/core/src/test/resources/sql-tests/results/pgSQL/date.sql.out

Lines 484 to 507 in a5a5da7

    
           -- !query 46 
        
           -- out of range 
        
           SELECT f1 - date '2000-01-01' AS `Days From 2K` FROM DATE_TBL 
        
           -- !query 46 schema 
        
           struct<Days From 2K:int> 
        
           -- !query 46 output 
        
           -1035 
        
           -1036 
        
           -1037 
        
           -1400 
        
           -1401 
        
           -1402 
        
           -1403 
        
           -15542 
        
           -15607 
        
           13977 
        
           14343 
        
           14710 
        
           91 
        
           92 
        
           93

After this PR:

spark/sql/core/src/test/resources/sql-tests/results/pgSQL/date.sql.out

Lines 482 to 501 in eeb7405

    
           -- !query 46 
        
           SELECT f1 - date '2000-01-01' AS `Days From 2K` FROM DATE_TBL 
        
           -- !query 46 schema 
        
           struct<Days From 2K:int> 
        
           -- !query 46 output 
        
           -1035 
        
           -1036 
        
           -1037 
        
           -1400 
        
           -1401 
        
           -1402 
        
           -1403 
        
           -15542 
        
           -15607 
        
           13977 
        
           14343 
        
           14710 
        
           91 
        
           92 
        
           93

How was this patch tested?

N/A

wangyum · 2019-08-05T03:29:49Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

    val input = fileToString(new File(testCase.inputFile))

-    val (comments, code) = input.split("\n").partition(_.startsWith("--"))
+    val (comments, codeMaybeWithComment) = input.split("\n").partition(_.trim.startsWith("--"))


Add trim to filter out these comments:

spark/sql/core/src/test/resources/sql-tests/inputs/subquery/in-subquery/not-in-unit-tests-multi-column.sql

Lines 32 to 33 in d440699

-- Case 1

-- (subquery is empty -> row is returned)

wangyum · 2019-08-05T03:36:03Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala


-    val (comments, code) = input.split("\n").partition(_.startsWith("--"))
+    val (comments, codeMaybeWithComment) = input.split("\n").partition(_.trim.startsWith("--"))
+    val code = codeMaybeWithComment.map(_.split("--").head)


Add split to fix these comments error:
Comment for SELECT date '5874898-01-01':

spark/sql/core/src/test/resources/sql-tests/inputs/pgSQL/date.sql

Line 200 in 2d74f14

SELECT date '5874898-01-01'; -- out of range

But the golden file is:

spark/sql/core/src/test/resources/sql-tests/results/pgSQL/date.sql.out

Lines 484 to 507 in a5a5da7

-- !query 46

-- out of range

SELECT f1 - date '2000-01-01' AS `Days From 2K` FROM DATE_TBL

-- !query 46 schema

struct<Days From 2K:int>

-- !query 46 output

-1035

-1036

-1037

-1400

-1401

-1402

-1403

-15542

-15607

13977

14343

14710

91

92

93

oh... really weird output... nice catch.

sql/core/src/test/resources/sql-tests/results/pgSQL/with.sql.out

dongjoon-hyun · 2019-08-05T04:48:27Z

Although there is a side effect, this PR seems to have a bigger positive side than a negative side. I'm supporting this approach, @wangyum .

cc @gatorsmile , @maropu , @HyukjinKwon

HyukjinKwon · 2019-08-05T06:18:22Z

sql/core/src/test/resources/sql-tests/results/udf/pgSQL/udf-aggregates_part2.sql.out

 -- !query 2
 SELECT
-  -- boolean or transitions
-  -- null because strict


Thanks for doing udf-* test cases too.

HyukjinKwon · 2019-08-05T06:20:31Z

+1 too

SparkQA · 2019-08-05T06:50:23Z

Test build #108639 has finished for PR 25357 at commit 523c426.

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

dongjoon-hyun · 2019-08-05T15:49:01Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala


-    val (comments, code) = input.split("\n").partition(_.startsWith("--"))
+    val (comments, codeMaybeWithComment) = input.split("\n").partition(_.trim.startsWith("--"))
+    val code = codeMaybeWithComment.map(_.split("--").head)


@wangyum . Is it safe with a query like SELECT '12--34'?

- val code = codeMaybeWithComment.map(_.split("--").head)

dongjoon-hyun

In this new approach, could you fix the new corner case like the following?

Input

SELECT '12--34'

Output

-- Automatically generated by SQLQueryTestSuite
-- Number of queries: 1


-- !query 0
SELECT '12
-- !query 0 schema
struct<>
-- !query 0 output
org.apache.spark.sql.catalyst.parser.ParseException

no viable alternative at input 'SELECT ''(line 1, pos 7)

== SQL ==
SELECT '12
-------^^^

maropu · 2019-08-06T04:19:13Z

+1, too

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

SparkQA · 2019-08-06T07:05:02Z

Test build #108695 has finished for PR 25357 at commit eeb7405.

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

SparkQA · 2019-08-06T13:55:40Z

Test build #108710 has finished for PR 25357 at commit a9abe00.

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

dongjoon-hyun · 2019-08-07T07:09:31Z

...ources/sql-tests/results/subquery/in-subquery/not-in-unit-tests-multi-column-literal.sql.out

 -- !query 1
-- Case 5
-  -- (one null column with no match -> row is returned)
+-- (one null column with no match -> row is returned)


Also, can we remove this too?

This will be removed by #25357 (comment)

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

SparkQA · 2019-08-07T11:46:27Z

Test build #108754 has finished for PR 25357 at commit 60e38af.

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

dongjoon-hyun

+1, LGTM. Thank you for fixing this, @wangyum .
Thank you, @HyukjinKwon and @maropu .
Merged to master.

wangyum added 2 commits August 5, 2019 10:14

Remove comment from golden files

3e7169f

Merge remote-tracking branch 'upstream/master' into comment

523c426

wangyum commented Aug 5, 2019

View reviewed changes

dongjoon-hyun reviewed Aug 5, 2019

View reviewed changes

sql/core/src/test/resources/sql-tests/results/pgSQL/with.sql.out Show resolved Hide resolved

dongjoon-hyun added SQL TESTS labels Aug 5, 2019

HyukjinKwon reviewed Aug 5, 2019

View reviewed changes

dongjoon-hyun reviewed Aug 5, 2019

View reviewed changes

dongjoon-hyun requested changes Aug 5, 2019

View reviewed changes

fix

eeb7405

wangyum changed the title ~~[SPARK-28617][SQL][TEST] Completely remove comments from the golden result file~~ [SPARK-28617][SQL][TEST] Fix misplacement when comment is at the end of the query Aug 6, 2019

maropu reviewed Aug 6, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala Show resolved Hide resolved

Add comment

a9abe00

maropu mentioned this pull request Aug 6, 2019

[SPARK-27924][SQL][TEST][FOLLOW-UP] Enable Boolean-Predicate syntax tests #25366

Closed

dongjoon-hyun reviewed Aug 7, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala Show resolved Hide resolved

wangyum added 2 commits August 7, 2019 15:47

Merge remote-tracking branch 'upstream/master' into SPARK-28617

b453dad

Address comment

60e38af

dongjoon-hyun approved these changes Aug 7, 2019

View reviewed changes

dongjoon-hyun closed this in eeaf185 Aug 7, 2019

wangyum deleted the SPARK-28617 branch August 8, 2019 01:01

maropu mentioned this pull request Aug 8, 2019

[SPARK-23160][SQL][TEST] Port window.sql #24881

Closed

HyukjinKwon mentioned this pull request Aug 12, 2019

[SPARK-28687][SQL] Support epoch, isoyear, milliseconds and microseconds at extract() #25408

Closed

	-- !query 46
	-- out of range



	SELECT f1 - date '2000-01-01' AS `Days From 2K` FROM DATE_TBL
	-- !query 46 schema
	struct<Days From 2K:int>
	-- !query 46 output
	-1035
	-1036
	-1037
	-1400
	-1401
	-1402
	-1403
	-15542
	-15607
	13977
	14343
	14710
	91
	92
	93

Conversation

wangyum commented Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

wangyum Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

wangyum Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

maropu Aug 6, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dongjoon-hyun commented Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyukjinKwon Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Aug 5, 2019

Uh oh!

SparkQA commented Aug 5, 2019

Uh oh!

dongjoon-hyun Aug 5, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

maropu commented Aug 6, 2019

Uh oh!

Uh oh!

SparkQA commented Aug 6, 2019

Uh oh!

SparkQA commented Aug 6, 2019

Uh oh!

dongjoon-hyun Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wangyum commented Aug 5, 2019 •

edited

Loading

dongjoon-hyun commented Aug 5, 2019 •

edited

Loading