You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2025/02/17 12:04:06 UTC
[PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
beliefer opened a new pull request, #49987:
URL: https://github.com/apache/spark/pull/49987
### What changes were proposed in this pull request?
This PR proposes to format the SqlBaseParser.g4
### Why are the changes needed?
Currently, some declaration in SqlBaseParser.g4 is very long that stretch out my laptop's screen. We have to slide the cursor or move the window.
This minor can increase the readability.
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
GA.
### Was this patch authored or co-authored using generative AI tooling?
'No'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2707398360
If there is no proper automation tool, I still prefer to keep it in the AS-IS status because this is controversial, @beliefer . However, I removed my request change on this PR. I'll leave this to the other community members. Thank you, @beliefer .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2665490401
@dongjoon-hyun @LuciferYang I can't find an automated tool to format or check the .g4 files. Do you have any other good idea ? or let us manually format .g4 files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #49987: [MINOR][SQL] Format the SqlBaseParser.g4
URL: https://github.com/apache/spark/pull/49987
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #49987:
URL: https://github.com/apache/spark/pull/49987#discussion_r1959584868
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -101,7 +101,8 @@ conditionValues
;
declareHandlerStatement
- : DECLARE (CONTINUE | EXIT) HANDLER FOR conditionValues (beginEndCompoundBlock | statement | setStatementWithOptionalVarKeyword)
+ : DECLARE (CONTINUE | EXIT) HANDLER FOR conditionValues
+ (beginEndCompoundBlock | statement | setStatementWithOptionalVarKeyword)
Review Comment:
Yes, we don't.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #49987:
URL: https://github.com/apache/spark/pull/49987#discussion_r1958773406
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -219,8 +220,8 @@ statement
ADD (COLUMN | COLUMNS)
columns=qualifiedColTypeWithPositionList #addTableColumns
| ALTER TABLE identifierReference
- ADD (COLUMN | COLUMNS)
- LEFT_PAREN columns=qualifiedColTypeWithPositionList RIGHT_PAREN #addTableColumns
+ ADD (COLUMN | COLUMNS) LEFT_PAREN
Review Comment:
Is this better? The previous pattern looks easier to read because `LEFT_PAREN and RIGHT_PAREN` are in the same line.
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -101,7 +101,8 @@ conditionValues
;
declareHandlerStatement
- : DECLARE (CONTINUE | EXIT) HANDLER FOR conditionValues (beginEndCompoundBlock | statement | setStatementWithOptionalVarKeyword)
+ : DECLARE (CONTINUE | EXIT) HANDLER FOR conditionValues
+ (beginEndCompoundBlock | statement | setStatementWithOptionalVarKeyword)
Review Comment:
May I ask where this coding guide comes from? IIUC, we don't have a code guide for `ANTLR` grammar file?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2663256574
I don't object to formatting this file, but are there any Maven/SBT plugins that can automate the formatting process? For example, `spotless-maven-plugin`? Or any others?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2662933859
ping @MaxGekk @dongjoon-hyun cc @yaooqinn
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2707785792
@dongjoon-hyun Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2665462267
@LuciferYang
spotless-maven-plugin doesn't support formatting .g4 files. It depends one the third-part module and it's not good enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][SQL] Format the SqlBaseParser.g4 [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49987:
URL: https://github.com/apache/spark/pull/49987#issuecomment-2978551298
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org