Support sql has comma before grouping sets#6402
Open
qian0817 wants to merge 1 commit intoalibaba:masterfrom
Open
Support sql has comma before grouping sets#6402qian0817 wants to merge 1 commit intoalibaba:masterfrom
qian0817 wants to merge 1 commit intoalibaba:masterfrom
Conversation
|
|
mengnankkkk
reviewed
Jul 12, 2025
| } | ||
|
|
||
| public boolean isGroupingSetsHaveComma() { | ||
| return groupingSetsHaveComma; |
There was a problem hiding this comment.
将“是否有逗号”作为 group by 子句的一个属性，虽然能解决当前需求，但这种语法细节直接暴露在 AST 层，可能导致后续 AST 结构膨胀，不利于维护和扩展。
建议：
逗号本质是语法格式，建议在 Visitor 层处理，而不是 AST 层增加属性。AST 层应只表达语义，不表达格式细节。
| item.addBeforeComment(comments); | ||
| } | ||
| if (item instanceof SQLGroupingSetExpr && hasComma) { | ||
| groupBy.setGroupingSetsHaveComma(true); |
There was a problem hiding this comment.
解析器通过 hasComma 标记逗号出现，进而设置 groupBy 的属性。这种做法会导致解析器和 AST 之间耦合过高，且对复杂 SQL（如嵌套、注释、换行）可能不够健壮。
| println(); | ||
| } | ||
| } else { | ||
| println(','); |
There was a problem hiding this comment.
Visitor 层根据 AST 属性决定是否输出逗号，导致 Visitor 依赖 AST 的格式属性，违背了 AST 只表达语义的设计原则。
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Both sql
SELECT brand, size, sum(sales) FROM items_sold GROUP BY brand, size, GROUPING SETS ((brand), (size), ());and
SELECT brand, size, sum(sales) FROM items_sold GROUP BY brand, size GROUPING SETS ((brand), (size), ());is allowed in many dbms like hive,spark,odps. we need to support them.