[
https://issues.apache.org/jira/browse/GROOVY-11919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072822#comment-18072822
]
ASF GitHub Bot commented on GROOVY-11919:
-----------------------------------------
Copilot commented on code in PR #2458:
URL: https://github.com/apache/groovy/pull/2458#discussion_r3067845637
##########
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java:
##########
@@ -110,30 +113,40 @@ public AbstractGinqExpression buildAST(ASTNode astNode) {
return getGinqExpression(astNode);
}
- private GinqExpression getGinqExpression(ASTNode astNode) {
+ private AbstractGinqExpression getGinqExpression(ASTNode astNode) {
if (null == latestGinqExpression) {
ASTNode node = ginqExpressionStack.isEmpty() ? astNode :
ginqExpressionStack.peek();
this.collectSyntaxError(new GinqSyntaxError("`select` clause is
missing",
node.getLineNumber(), node.getColumnNumber()));
}
- latestGinqExpression.visit(new GinqAstBaseVisitor() {
+ AbstractGinqExpression result;
+ if (setOperationLeft != null && latestGinqExpression != null) {
+ latestGinqExpression.putNodeMetaData(ROOT_GINQ_EXPRESSION,
latestGinqExpression);
+ SetOperationExpression setOpExpr = new
SetOperationExpression(setOperationLeft, setOperationOp, latestGinqExpression);
+ setOpExpr.setSourcePosition(setOperationLeft);
+ result = setOpExpr;
+ } else {
+ latestGinqExpression.putNodeMetaData(ROOT_GINQ_EXPRESSION,
latestGinqExpression);
+ result = latestGinqExpression;
+ }
Review Comment:
`getGinqExpression` can throw a NullPointerException when a set operation is
started but the right-hand query is missing (e.g. the script ends after
`union`). In that situation `latestGinqExpression` is null but the code still
calls `latestGinqExpression.putNodeMetaData(...)`. Consider short-circuiting
when `latestGinqExpression` is null (and reporting a dedicated syntax error
like “right-hand query for set operation is missing”).
##########
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java:
##########
@@ -420,6 +433,27 @@ public void visitBinaryExpression(BinaryExpression
expression) {
@Override
public void visitVariableExpression(VariableExpression expression) {
+ if (SET_OP_SET.contains(expression.getText())) {
+ if (latestGinqExpression == null) {
+ this.collectSyntaxError(new GinqSyntaxError(
+ "`" + expression.getText() + "` must follow a complete
GINQ expression (from...select)",
+ expression.getLineNumber(),
expression.getColumnNumber()));
+ } else {
+ if (setOperationLeft != null) {
+ // chaining: wrap previous left + op + current right into
a SetOperationExpression
+ latestGinqExpression.putNodeMetaData(ROOT_GINQ_EXPRESSION,
latestGinqExpression);
+ SetOperationExpression prev = new
SetOperationExpression(setOperationLeft, setOperationOp, latestGinqExpression);
+ prev.setSourcePosition(setOperationLeft);
+ setOperationLeft = prev;
+ } else {
+ setOperationLeft = latestGinqExpression;
+ }
+ setOperationOp = expression.getText();
+ latestGinqExpression = null;
Review Comment:
When parsing the first set operator, `setOperationLeft =
latestGinqExpression` does not mark that left query as `ROOT_GINQ_EXPRESSION`.
As a result, in `GinqAstWalker.visitGinqExpression` the left query won’t be
treated as “root” and won’t enable window-function/parallel scaffolding even if
it uses `over(...)` or parallel config. Consider setting
`latestGinqExpression.putNodeMetaData(ROOT_GINQ_EXPRESSION,
latestGinqExpression)` before assigning it to `setOperationLeft` (and similarly
ensure each top-level query in a set-op chain is tagged).
> Provide set operators for GINQ, e.g. union, intersect
> -----------------------------------------------------
>
> Key: GROOVY-11919
> URL: https://issues.apache.org/jira/browse/GROOVY-11919
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
