[SPARK-31262][SQL][TESTS] Fix bug tests imported bracketed comments by beliefer · Pull Request #28018 · apache/spark

beliefer · 2020-03-25T09:04:52Z

What changes were proposed in this pull request?

This PR related to #27481.
If test case A uses --IMPORT to import test case B contains bracketed comments, the output can't display bracketed comments in golden files well.
The content of nested-comments.sql show below:

-- This test case just used to test imported bracketed comments.

-- the first case of bracketed comment
--QUERY-DELIMITER-START
/* This is the first example of bracketed comment.
SELECT 'ommented out content' AS first;
*/
SELECT 'selected content' AS first;
--QUERY-DELIMITER-END

The test case comments.sql imports nested-comments.sql below:
--IMPORT nested-comments.sql
Before this PR, the output will be:

-- !query
/* This is the first example of bracketed comment.
SELECT 'ommented out content' AS first
-- !query schema
struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException

mismatched input '/' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 
'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', '
ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 0)

== SQL ==
/* This is the first example of bracketed comment.
^^^
SELECT 'ommented out content' AS first


-- !query
*/
SELECT 'selected content' AS first
-- !query schema
struct<>
-- !query output
org.apache.spark.sql.catalyst.parser.ParseException

extraneous input '*/' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', 'ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 0)

== SQL ==
*/
^^^
SELECT 'selected content' AS first

After this PR, the output will be:

-- !query
/* This is the first example of bracketed comment.
SELECT 'ommented out content' AS first;
*/
SELECT 'selected content' AS first
-- !query schema
struct<first:string>
-- !query output
selected content

Why are the changes needed?

Golden files can't display the bracketed comments in imported test cases.

Does this PR introduce any user-facing change?

'No'.

How was this patch tested?

New UT.

beliefer · 2020-03-25T13:55:12Z

cc @cloud-fan @maropu

SparkQA · 2020-03-25T15:07:04Z

Test build #120329 has finished for PR 28018 at commit bbb9a6a.

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

SparkQA · 2020-03-25T17:19:11Z

Test build #120341 has finished for PR 28018 at commit cec4670.

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

maropu · 2020-03-25T22:52:09Z

sql/core/src/test/resources/sql-tests/inputs/comments.sql

-*/
-SELECT 'selected content' AS first;
--QUERY-DELIMITER-END
+--IMPORT nested-comments.sql


What did this means?

Golden files can't display well.

I updated the description to Golden files can't display the bracketed comments in imported test cases.

Also, could you put an output example before/after this PR in the PR description?

OK. I have put the output example

maropu · 2020-03-25T22:53:25Z

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 { line =>
+    def splitCommentsAndCodes(input: String) = input.split("\n").partition { line =>


Is this fix related to this PR?

maropu · 2020-03-26T01:26:23Z

I think this issue is worth filing a new jira.

beliefer · 2020-03-26T03:25:27Z

I think this issue is worth filing a new jira.

OK. I created a new JIRA and update the title of this PR.

cloud-fan · 2020-03-26T07:29:40Z

sql/core/src/test/resources/sql-tests/inputs/nested-comments.sql

+-- the first case of bracketed comment
+--QUERY-DELIMITER-START
+/* This is the first example of bracketed comment.
+SELECT 'ommented out content' AS first;


This is not a nested comment, right?

This is a bracketed comments.
Maybe I need change nested-comments.sql to imported-comments.sql?

maybe updating SQLQueryTestSuite only is good enough. It's an obvious bug that we forget to handle QUERY-DELIMITER in imported code.

You means I shouldn't change comments.sql and add nested-comments.sql ?

yea, it's not necessary to separate this test file.

OK. I will revert it.

SparkQA · 2020-03-26T16:41:37Z

Test build #120419 has finished for PR 28018 at commit fddcbe0.

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

SparkQA · 2020-03-26T17:27:23Z

Test build #120420 has finished for PR 28018 at commit 1e60d83.

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

### What changes were proposed in this pull request? This PR related to #27481. If test case A uses `--IMPORT` to import test case B contains bracketed comments, the output can't display bracketed comments in golden files well. The content of `nested-comments.sql` show below: ``` -- This test case just used to test imported bracketed comments. -- the first case of bracketed comment --QUERY-DELIMITER-START /* This is the first example of bracketed comment. SELECT 'ommented out content' AS first; */ SELECT 'selected content' AS first; --QUERY-DELIMITER-END ``` The test case `comments.sql` imports `nested-comments.sql` below: `--IMPORT nested-comments.sql` Before this PR, the output will be: ``` -- !query /* This is the first example of bracketed comment. SELECT 'ommented out content' AS first -- !query schema struct<> -- !query output org.apache.spark.sql.catalyst.parser.ParseException mismatched input '/' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', ' ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 0) == SQL == /* This is the first example of bracketed comment. ^^^ SELECT 'ommented out content' AS first -- !query */ SELECT 'selected content' AS first -- !query schema struct<> -- !query output org.apache.spark.sql.catalyst.parser.ParseException extraneous input '*/' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', 'ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 0) == SQL == */ ^^^ SELECT 'selected content' AS first ``` After this PR, the output will be: ``` -- !query /* This is the first example of bracketed comment. SELECT 'ommented out content' AS first; */ SELECT 'selected content' AS first -- !query schema struct<first:string> -- !query output selected content ``` ### Why are the changes needed? Golden files can't display the bracketed comments in imported test cases. ### Does this PR introduce any user-facing change? 'No'. ### How was this patch tested? New UT. Closes #28018 from beliefer/fix-bug-tests-imported-bracketed-comments. Authored-by: beliefer <beliefer@163.com> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org> (cherry picked from commit 9e0fee9) Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

maropu · 2020-03-26T23:10:02Z

Thanks! Merged to master/3.0.

beliefer · 2020-03-27T00:11:50Z

@maropu @cloud-fan @dongjoon-hyun Thanks for all.

### What changes were proposed in this pull request? This PR related to apache#27481. If test case A uses `--IMPORT` to import test case B contains bracketed comments, the output can't display bracketed comments in golden files well. The content of `nested-comments.sql` show below: ``` -- This test case just used to test imported bracketed comments. -- the first case of bracketed comment --QUERY-DELIMITER-START /* This is the first example of bracketed comment. SELECT 'ommented out content' AS first; */ SELECT 'selected content' AS first; --QUERY-DELIMITER-END ``` The test case `comments.sql` imports `nested-comments.sql` below: `--IMPORT nested-comments.sql` Before this PR, the output will be: ``` -- !query /* This is the first example of bracketed comment. SELECT 'ommented out content' AS first -- !query schema struct<> -- !query output org.apache.spark.sql.catalyst.parser.ParseException mismatched input '/' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', ' ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 0) == SQL == /* This is the first example of bracketed comment. ^^^ SELECT 'ommented out content' AS first -- !query */ SELECT 'selected content' AS first -- !query schema struct<> -- !query output org.apache.spark.sql.catalyst.parser.ParseException extraneous input '*/' expecting {'(', 'ADD', 'ALTER', 'ANALYZE', 'CACHE', 'CLEAR', 'COMMENT', 'COMMIT', 'CREATE', 'DELETE', 'DESC', 'DESCRIBE', 'DFS', 'DROP', 'EXPLAIN', 'EXPORT', 'FROM', 'GRANT', 'IMPORT', 'INSERT', 'LIST', 'LOAD', 'LOCK', 'MAP', 'MERGE', 'MSCK', 'REDUCE', 'REFRESH', 'REPLACE', 'RESET', 'REVOKE', 'ROLLBACK', 'SELECT', 'SET', 'SHOW', 'START', 'TABLE', 'TRUNCATE', 'UNCACHE', 'UNLOCK', 'UPDATE', 'USE', 'VALUES', 'WITH'}(line 1, pos 0) == SQL == */ ^^^ SELECT 'selected content' AS first ``` After this PR, the output will be: ``` -- !query /* This is the first example of bracketed comment. SELECT 'ommented out content' AS first; */ SELECT 'selected content' AS first -- !query schema struct<first:string> -- !query output selected content ``` ### Why are the changes needed? Golden files can't display the bracketed comments in imported test cases. ### Does this PR introduce any user-facing change? 'No'. ### How was this patch tested? New UT. Closes apache#28018 from beliefer/fix-bug-tests-imported-bracketed-comments. Authored-by: beliefer <beliefer@163.com> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

dongjoon-hyun added SQL TESTS labels Mar 25, 2020

maropu reviewed Mar 25, 2020

View reviewed changes

beliefer changed the title ~~[SPARK-30758][SQL][TESTS][FOLLOWUP] Fix bug tests imported bracketed comments~~ [SPARK-31262][SQL][TESTS][FOLLOWUP] Fix bug tests imported bracketed comments Mar 26, 2020

cloud-fan reviewed Mar 26, 2020

View reviewed changes

maropu changed the title ~~[SPARK-31262][SQL][TESTS][FOLLOWUP] Fix bug tests imported bracketed comments~~ [SPARK-31262][SQL][TESTS] Fix bug tests imported bracketed comments Mar 26, 2020

beliefer force-pushed the fix-bug-tests-imported-bracketed-comments branch from cec4670 to fddcbe0 Compare March 26, 2020 12:03

Revert test case.

1e60d83

beliefer force-pushed the fix-bug-tests-imported-bracketed-comments branch from fddcbe0 to 1e60d83 Compare March 26, 2020 12:14

cloud-fan approved these changes Mar 26, 2020

View reviewed changes

maropu approved these changes Mar 26, 2020

View reviewed changes

maropu closed this in 9e0fee9 Mar 26, 2020

beliefer deleted the fix-bug-tests-imported-bracketed-comments branch April 23, 2024 07:01

Comments

Conversation

beliefer commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

beliefer commented Mar 25, 2020

Uh oh!

SparkQA commented Mar 25, 2020

Uh oh!

SparkQA commented Mar 25, 2020

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!

maropu commented Mar 26, 2020

Uh oh!

beliefer commented Mar 26, 2020

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!

SparkQA commented Mar 26, 2020

Uh oh!

SparkQA commented Mar 26, 2020

Uh oh!

maropu commented Mar 26, 2020

Uh oh!

beliefer commented Mar 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

beliefer commented Mar 25, 2020 •

edited

Loading