[SPARK-33100][SQL][2.4] Ignore a semicolon inside a bracketed comment in spark-sql #31040
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ithin quoted string literals This pr port [HIVE-15297](https://issues.apache.org/jira/browse/HIVE-15297) to fix **spark-sql** should not split semicolon within quoted string literals. unit tests and manual tests:  Closes apache#25018 from wangyum/SPARK-26321. Authored-by: Yuming Wang <[email protected]> Signed-off-by: Yuming Wang <[email protected]>
…hed quote character
A SQL statement that contains a comment with an unmatched quote character can lead to a parse error:
- Added a insideComment flag in the splitter method to avoid checking single and double quotes within a comment:
```
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 misbehaviour was not present on previous spark versions.
- No
- New tests were added.
Closes apache#27321 from javierivanov/SPARK-30049B.
Lead-authored-by: Javier <[email protected]>
Co-authored-by: Javier Fuentes <[email protected]>
Signed-off-by: Thomas Graves <[email protected]>
…ted string ### What changes were proposed in this pull request? `def splitSemiColon` cannot handle unescaped quote mark like "'" or '"' correctly. When there are unmatched quotes in a string, `splitSemiColon` will not drop off semicolon as expected. ### Why are the changes needed? Some regex expression will use quote mark in string. We should process semicolon correctly. ### Does this PR introduce any user-facing change? No ### How was this patch tested? Added Unit test and also manual test. Closes apache#28393 from adrian-wang/unescaped. Authored-by: Daoyuan Wang <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
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 <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
Member
Author
|
gentle ping @maropu |
|
Can one of the admins verify this patch? |
…park-sql Now the spark-sql does not support parse the sql statements with bracketed comments. For the sql statements: ``` /* SELECT 'test'; */ SELECT 'test'; ``` Would be split to two statements: The first one: `/* SELECT 'test'` The second one: `*/ SELECT 'test'` Then it would throw an exception because the first one is illegal. In this PR, we ignore the content in bracketed comments while splitting the sql statements. Besides, we ignore the comment without any content. Spark-sql might split the statements inside bracketed comments and it is not correct. No. Added UT. Closes apache#29982 from turboFei/SPARK-33110. Lead-authored-by: fwang12 <[email protected]> Co-authored-by: turbofei <[email protected]> Signed-off-by: Takeshi Yamamuro <[email protected]>
turboFei
force-pushed
the
SPARK-33100-2.4
branch
from
dc98763 to
851a989
Compare
turboFei
commented
Jan 5, 2021
| runCliWithin(2.minute)( | ||
| "CREATE TEMPORARY VIEW t AS SELECT * FROM VALUES(1, 2) AS t(k, v);" -> "", | ||
| "EXPLAIN EXTENDED SELECT /*+ broadcast(t) */ * from t;" | ||
| -> "ResolvedHint (strategy=broadcast)" |
Member
Author
There was a problem hiding this comment.
because spark-2.4 does not support mergeJoin hint, so I modify this ut
Member
|
I will close this for now and see the comment: #31033 (comment) |
maropu
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport below PR as dependencies:
#25018
#27321
#28393
#27920