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-31102][SQL][3.0] Spark-sql fails to parse when contains comment #28565

Closed
wants to merge 2 commits into from

Conversation

javierivanov
Copy link
Contributor

This PR introduces a change to false for the insideComment flag on a newline. Fixing the issue introduced by SPARK-30049.

Backport to 3.0 from #27920

Previously on SPARK-30049 a comment containing an unclosed quote produced the following issue:

spark-sql> SELECT 1 -- someone's comment here
         > ;
Error in query:
extraneous input ';' expecting <EOF>(line 2, pos 0)

== SQL ==
SELECT 1 -- someone's comment here
;
^^^

This was caused because there was no flag for comment sections inside the splitSemiColon method to ignore quotes. SPARK-30049 added that flag and fixed the issue, but introduced the follwoing problem:

spark-sql> select
         >   1,
         >   -- two
         >   2;
Error in query:
mismatched input '<EOF>' expecting {'(', 'ADD', 'AFTER', 'ALL', 'ALTER', ...}(line 3, pos 2)
== SQL ==
select
  1,
--^^^

This issue is generated by a missing turn-off for the insideComment flag with a newline.

No

  • For previous tests using line-continuity(\) it was added a line-continuity rule in the SqlBase.g4 file to add the functionality to the SQL context.
  • A new test for inline comments was added.

Closes #27920 from javierivanov/SPARK-31102.

Authored-by: Javier Fuentes j.fuentes.m@icloud.com
Signed-off-by: Wenchen Fan wenchen@databricks.com

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?

This PR introduces a change to false for the insideComment flag on a newline. Fixing the issue introduced by SPARK-30049.

Previously on SPARK-30049 a comment containing an unclosed quote produced the following issue:
```
spark-sql> SELECT 1 -- someone's comment here
         > ;
Error in query:
extraneous input ';' expecting <EOF>(line 2, pos 0)

== SQL ==
SELECT 1 -- someone's comment here
;
^^^
```

This was caused because there was no flag for comment sections inside the splitSemiColon method to ignore quotes. SPARK-30049 added that flag and fixed the issue, but introduced the follwoing problem:
```
spark-sql> select
         >   1,
         >   -- two
         >   2;
Error in query:
mismatched input '<EOF>' expecting {'(', 'ADD', 'AFTER', 'ALL', 'ALTER', ...}(line 3, pos 2)
== SQL ==
select
  1,
--^^^
```
This issue is generated by a missing turn-off for the insideComment flag with a newline.

No

- For previous tests using line-continuity(`\`) it was added a line-continuity rule in the SqlBase.g4 file to add the functionality to the SQL context.
- A new test for inline comments was added.

Closes apache#27920 from javierivanov/SPARK-31102.

Authored-by: Javier Fuentes <j.fuentes.m@icloud.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@cloud-fan
Copy link
Contributor

ok to test

@SparkQA
Copy link

SparkQA commented May 18, 2020

Test build #122783 has finished for PR 28565 at commit 49fa2a8.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member

maropu commented May 18, 2020

retest this please

@SparkQA
Copy link

SparkQA commented May 18, 2020

Test build #122788 has finished for PR 28565 at commit 49fa2a8.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member

maropu commented May 18, 2020

It seems the failure above is not related to this PR. See: #28566

@HyukjinKwon
Copy link
Member

retest this please

@maropu
Copy link
Member

maropu commented May 18, 2020

Looks nice! Thanks for re-trigger the tests, @HyukjinKwon .

@SparkQA
Copy link

SparkQA commented May 18, 2020

Test build #122789 has finished for PR 28565 at commit 49fa2a8.

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

@AngersZhuuuu
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented May 18, 2020

Test build #122795 has finished for PR 28565 at commit 49fa2a8.

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

@cloud-fan
Copy link
Contributor

The newly added test all fails. Seems there is something different in branch 3.0.

@javierivanov
Copy link
Contributor Author

The newly added test all fails. Seems there is something different in branch 3.0.

I added those test by mistake while cherry-picking. Checking.

@SparkQA
Copy link

SparkQA commented May 18, 2020

Test build #122805 has finished for PR 28565 at commit a318127.

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

maropu pushed a commit that referenced this pull request May 18, 2020
This PR introduces a change to false for the insideComment flag on a newline. Fixing the issue introduced by SPARK-30049.

Backport to 3.0 from #27920

Previously on SPARK-30049 a comment containing an unclosed quote produced the following issue:
```
spark-sql> SELECT 1 -- someone's comment here
         > ;
Error in query:
extraneous input ';' expecting <EOF>(line 2, pos 0)

== SQL ==
SELECT 1 -- someone's comment here
;
^^^
```

This was caused because there was no flag for comment sections inside the splitSemiColon method to ignore quotes. SPARK-30049 added that flag and fixed the issue, but introduced the follwoing problem:
```
spark-sql> select
         >   1,
         >   -- two
         >   2;
Error in query:
mismatched input '<EOF>' expecting {'(', 'ADD', 'AFTER', 'ALL', 'ALTER', ...}(line 3, pos 2)
== SQL ==
select
  1,
--^^^
```
This issue is generated by a missing turn-off for the insideComment flag with a newline.

No

- For previous tests using line-continuity(`\`) it was added a line-continuity rule in the SqlBase.g4 file to add the functionality to the SQL context.
- A new test for inline comments was added.

Closes #27920 from javierivanov/SPARK-31102.

Authored-by: Javier Fuentes <j.fuentes.micloud.com>
Signed-off-by: Wenchen Fan <wenchendatabricks.com>

### 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?

Closes #28565 from javierivanov/SPARK-3.0-31102.

Authored-by: Javier Fuentes <j.fuentes.m@icloud.com>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
@maropu
Copy link
Member

maropu commented May 18, 2020

Thanks! Merged to branch-3.0.

@maropu maropu closed this May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants