You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2021/09/23 22:55:40 UTC

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

From Dmitry Lychagin <dm...@couchbase.com>:

Dmitry Lychagin has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )


Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................

[NO ISSUE][COMP] Refactor Projection clause

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- Introduce Projection.Kind for projection kinds
  and use it instead of boolean flags
- OperatorExpr.opIsComparison() should return true
  for DISTINCT/NOT_DISTINCT operators
- Add missing setters to SelectClause and SelectExpression

Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
---
M asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
19 files changed, 182 insertions(+), 90 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/64/13364/1

diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
index a37a366..3deedfd 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
@@ -77,6 +77,8 @@
             case GE:
             case LT:
             case LE:
+            case DISTINCT:
+            case NOT_DISTINCT:
                 return true;
             default:
                 return false;
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
index 7f353d6..7aa3cec 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
@@ -29,16 +29,56 @@
 
 public class Projection extends AbstractClause {
 
+    public enum Kind {
+        NAMED_EXPR, // expr AS name
+        STAR, // *
+        VAR_STAR // variable.*
+    }
+
+    private Kind kind;
     private Expression expr;
     private String name;
-    private boolean star;
-    private boolean varStar;
 
-    public Projection(Expression expr, String name, boolean star, boolean varStar) {
+    public Projection(Kind kind, Expression expr, String name) {
+        validateKind(kind, expr, name);
+        this.kind = kind;
         this.expr = expr;
         this.name = name;
-        this.star = star;
-        this.varStar = varStar;
+    }
+
+    private static void validateKind(Kind kind, Expression expr, String name) {
+        switch (kind) {
+            case NAMED_EXPR:
+                if (expr == null) {
+                    throw new NullPointerException();
+                }
+                break;
+            case STAR:
+                if (expr != null || name != null) {
+                    throw new IllegalArgumentException();
+                }
+                break;
+            case VAR_STAR:
+                if (expr == null) {
+                    throw new NullPointerException();
+                }
+                if (name != null) {
+                    throw new IllegalArgumentException();
+                }
+                break;
+            default:
+                throw new IllegalArgumentException();
+        }
+    }
+
+    @Deprecated
+    public Projection(Expression expr, String name, boolean star, boolean varStar) {
+        this(asKind(star, varStar), expr, name);
+    }
+
+    @Deprecated
+    private static Kind asKind(boolean star, boolean varStar) {
+        return star ? Kind.STAR : varStar ? Kind.VAR_STAR : Kind.NAMED_EXPR;
     }
 
     @Override
@@ -59,6 +99,10 @@
         this.expr = expr;
     }
 
+    public boolean hasExpression() {
+        return expr != null;
+    }
+
     public String getName() {
         return name;
     }
@@ -71,22 +115,41 @@
         return name != null;
     }
 
-    public boolean star() {
-        return star;
+    public Kind getKind() {
+        return kind;
     }
 
+    public void setKind(Kind kind) {
+        this.kind = kind;
+    }
+
+    @Deprecated
+    public boolean star() {
+        return kind == Kind.STAR;
+    }
+
+    @Deprecated
     public boolean varStar() {
-        return varStar;
+        return kind == Kind.VAR_STAR;
     }
 
     @Override
     public String toString() {
-        return star ? "*" : (String.valueOf(expr) + (varStar ? ".*" : (hasName() ? " as " + getName() : "")));
+        switch (kind) {
+            case NAMED_EXPR:
+                return expr + (hasName() ? " as " + getName() : "");
+            case STAR:
+                return "*";
+            case VAR_STAR:
+                return expr + ".*";
+            default:
+                throw new IllegalStateException();
+        }
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(expr, varStar, name, star);
+        return Objects.hash(kind, expr, name);
     }
 
     @Override
@@ -98,7 +161,6 @@
             return false;
         }
         Projection target = (Projection) object;
-        return Objects.equals(expr, target.expr) && Objects.equals(name, target.name) && varStar == target.varStar
-                && star == target.star;
+        return kind == target.kind && Objects.equals(expr, target.expr) && Objects.equals(name, target.name);
     }
 }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
index 3b550a3..ea6bac9 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
@@ -48,6 +48,16 @@
         return ClauseType.SELECT_CLAUSE;
     }
 
+    public void setSelectElement(SelectElement selectElement) {
+        this.selectElement = selectElement;
+        this.selectRegular = null;
+    }
+
+    public void setSelectRegular(SelectRegular selectRegular) {
+        this.selectRegular = selectRegular;
+        this.selectElement = null;
+    }
+
     public SelectElement getSelectElement() {
         return selectElement;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
index 62b8734..2a3ec4e 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
@@ -68,10 +68,18 @@
         return selectSetOperation;
     }
 
+    public void setOrderbyClause(OrderbyClause orderbyClause) {
+        this.orderbyClause = orderbyClause;
+    }
+
     public OrderbyClause getOrderbyClause() {
         return orderbyClause;
     }
 
+    public void setLimitClause(LimitClause limitClause) {
+        this.limitClause = limitClause;
+    }
+
     public LimitClause getLimitClause() {
         return limitClause;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
index 3067641..e488e9d 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
@@ -67,7 +67,7 @@
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (!projection.star() && !projection.varStar() && projection.getName() == null) {
+        if (projection.getKind() == Projection.Kind.NAMED_EXPR && !projection.hasName()) {
             projection.setName(SqlppVariableUtil.variableNameToDisplayedFieldName(context.newVariable().getValue()));
         }
         return super.visit(projection, arg);
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
index 416f49b..53cd6de 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
@@ -151,7 +151,7 @@
     private Map<Expression, ColumnAliasBinding> mapProjections(List<Projection> projections) {
         Map<Expression, ColumnAliasBinding> exprMap = new HashMap<>();
         for (Projection projection : projections) {
-            if (!projection.star() && !projection.varStar()) {
+            if (projection.getKind() == Projection.Kind.NAMED_EXPR) {
                 String varName = SqlppVariableUtil.toInternalVariableName(projection.getName());
                 exprMap.put(new VariableExpr(new VarIdentifier(varName)), ColumnAliasBinding.of(projection));
             }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
index 8559c96..592d3f6 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
@@ -89,10 +89,7 @@
 
     @Override
     public Void visit(Projection projection, Void arg) throws CompilationException {
-        if (!projection.star()) {
-            projection.getExpression().accept(this, arg);
-        }
-        return null;
+        return projection.hasExpression() ? projection.getExpression().accept(this, arg) : null;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
index 791bfe1..9aca37a 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
@@ -177,33 +177,43 @@
         List<VariableExpr> gbyBindingVars = null, postGbyBindingVars = null;
         for (VariableExpr freeVarPostSetOp : freeVarsPostSetOp) {
             String projectionName = null;
-            for (int i = projectionList.size() - 1; i >= 0; i--) {
+            inner_loop: for (int i = projectionList.size() - 1; i >= 0; i--) {
                 Projection projection = projectionList.get(i);
-                if (projection.varStar()) {
-                    throw new CompilationException(ErrorCode.UNSUPPORTED_GBY_OBY_SELECT_COMBO,
-                            selectBlock.getSourceLocation());
-                } else if (projection.star()) {
-                    if (gbyBindingVars == null) {
-                        gbyBindingVars = SqlppVariableUtil.getBindingVariables(selectBlock.getGroupbyClause());
-                    }
-                    if (postGbyBindingVars == null) {
-                        postGbyBindingVars = selectBlock.hasLetHavingClausesAfterGroupby()
-                                ? SqlppVariableUtil.getLetBindingVariables(selectBlock.getLetHavingListAfterGroupby())
-                                : Collections.emptyList();
-                    }
-                    if (gbyBindingVars.contains(freeVarPostSetOp) || postGbyBindingVars.contains(freeVarPostSetOp)) {
-                        projectionName = SqlppVariableUtil
-                                .variableNameToDisplayedFieldName(freeVarPostSetOp.getVar().getValue());
+                switch (projection.getKind()) {
+                    case VAR_STAR:
+                        throw new CompilationException(ErrorCode.UNSUPPORTED_GBY_OBY_SELECT_COMBO,
+                                selectBlock.getSourceLocation());
+                    case STAR:
+                        if (gbyBindingVars == null) {
+                            gbyBindingVars = SqlppVariableUtil.getBindingVariables(selectBlock.getGroupbyClause());
+                        }
+                        if (postGbyBindingVars == null) {
+                            postGbyBindingVars =
+                                    selectBlock.hasLetHavingClausesAfterGroupby()
+                                            ? SqlppVariableUtil
+                                                    .getLetBindingVariables(selectBlock.getLetHavingListAfterGroupby())
+                                            : Collections.emptyList();
+                        }
+                        if (gbyBindingVars.contains(freeVarPostSetOp)
+                                || postGbyBindingVars.contains(freeVarPostSetOp)) {
+                            projectionName = SqlppVariableUtil
+                                    .variableNameToDisplayedFieldName(freeVarPostSetOp.getVar().getValue());
+                            break;
+                        }
                         break;
-                    }
-                } else if (projection.hasName()) {
-                    if (projection.getExpression().equals(freeVarPostSetOp)) {
-                        projectionName = projection.getName();
+                    case NAMED_EXPR:
+                        if (!projection.hasName()) {
+                            throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE,
+                                    selectBlock.getSourceLocation(), "");
+                        }
+                        if (projection.getExpression().equals(freeVarPostSetOp)) {
+                            projectionName = projection.getName();
+                            break inner_loop;
+                        }
                         break;
-                    }
-                } else {
-                    throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, selectBlock.getSourceLocation(),
-                            "");
+                    default:
+                        throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE,
+                                selectBlock.getSourceLocation(), "");
                 }
             }
 
@@ -396,7 +406,8 @@
         }
         SelectElement selectElement = selectClause.getSelectElement();
         List<Projection> projectionList = new ArrayList<>(1);
-        projectionList.add(new Projection(selectElement.getExpression(), mainProjectionName, false, false));
+        projectionList
+                .add(new Projection(Projection.Kind.NAMED_EXPR, selectElement.getExpression(), mainProjectionName));
         SelectRegular newSelectRegular = new SelectRegular(projectionList);
         newSelectRegular.setSourceLocation(selectElement.getSourceLocation());
         SelectClause newSelectClause = new SelectClause(null, newSelectRegular, selectClause.distinct());
@@ -412,8 +423,8 @@
         }
         SelectRegular selectRegular = selectClause.getSelectRegular();
         for (Map.Entry<VariableExpr, String> me : projections.entrySet()) {
-            Projection newProjection =
-                    new Projection((VariableExpr) SqlppRewriteUtil.deepCopy(me.getKey()), me.getValue(), false, false);
+            Projection newProjection = new Projection(Projection.Kind.NAMED_EXPR,
+                    (VariableExpr) SqlppRewriteUtil.deepCopy(me.getKey()), me.getValue());
             selectRegular.getProjections().add(newProjection);
         }
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
index 56fdd19..9f93363 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
@@ -119,7 +119,7 @@
 
     @Override
     public Boolean visit(Projection projection, Void arg) throws CompilationException {
-        if (projection.star()) {
+        if (!projection.hasExpression()) {
             return false;
         }
         Pair<Boolean, Expression> p = inlineUdfsAndViewsInExpr(projection.getExpression());
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
index a8db06f..8a3b913 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
@@ -370,7 +370,8 @@
     }
 
     private Projection createProjection(VariableExpr var, String fieldName, SourceLocation sourceLoc) {
-        Projection projection = new Projection(newVariableExpr(var.getVar(), null), fieldName, false, false);
+        Projection projection =
+                new Projection(Projection.Kind.NAMED_EXPR, newVariableExpr(var.getVar(), null), fieldName);
         projection.setSourceLocation(sourceLoc);
         return projection;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
index 027560c..3192541 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
@@ -240,10 +240,7 @@
 
     @Override
     public Boolean visit(Projection projection, ILangExpression parentSelectBlock) throws CompilationException {
-        if (projection.star()) {
-            return false;
-        }
-        return projection.getExpression().accept(this, parentSelectBlock);
+        return projection.hasExpression() ? projection.getExpression().accept(this, parentSelectBlock) : false;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
index bc31e62..a903279 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
@@ -107,10 +107,7 @@
 
     @Override
     public Boolean visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (projection.star()) {
-            return false;
-        }
-        return visit(projection.getExpression(), arg);
+        return projection.hasExpression() && visit(projection.getExpression(), arg);
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
index ec6de6e..283b416 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
@@ -143,9 +143,9 @@
 
     @Override
     public Projection visit(Projection projection, Void arg) throws CompilationException {
-        Projection copy =
-                new Projection(projection.star() ? null : (Expression) projection.getExpression().accept(this, arg),
-                        projection.getName(), projection.star(), projection.varStar());
+        Projection copy = new Projection(projection.getKind(),
+                projection.hasExpression() ? (Expression) projection.getExpression().accept(this, arg) : null,
+                projection.getName());
         copy.setSourceLocation(projection.getSourceLocation());
         return copy;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
index 0c14688..77b9991 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
@@ -146,10 +146,7 @@
 
     @Override
     public Void visit(Projection projection, Collection<VariableExpr> freeVars) throws CompilationException {
-        if (!projection.star()) {
-            projection.getExpression().accept(this, freeVars);
-        }
-        return null;
+        return projection.hasExpression() ? projection.getExpression().accept(this, freeVars) : null;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
index 90e7448..27c3952 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
@@ -131,11 +131,18 @@
 
     @Override
     public Void visit(Projection projection, Integer step) throws CompilationException {
-        if (projection.star()) {
-            out.println(skip(step) + "*");
-        } else {
-            projection.getExpression().accept(this, step);
-            out.println(skip(step) + (projection.varStar() ? ".*" : projection.getName()));
+        switch (projection.getKind()) {
+            case STAR:
+                out.println(skip(step) + "*");
+                break;
+            case VAR_STAR:
+                projection.getExpression().accept(this, step);
+                out.println(skip(step) + ".*");
+                break;
+            case NAMED_EXPR:
+                projection.getExpression().accept(this, step);
+                out.println(skip(step) + projection.getName());
+                break;
         }
         return null;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
index bd64314..e664d49 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
@@ -202,11 +202,9 @@
     @Override
     public Pair<ILangExpression, VariableSubstitutionEnvironment> visit(Projection projection,
             VariableSubstitutionEnvironment env) throws CompilationException {
-        if (projection.star()) {
-            return new Pair<>(projection, env);
-        }
-        Projection newProjection = new Projection((Expression) projection.getExpression().accept(this, env).first,
-                projection.getName(), projection.star(), projection.varStar());
+        Projection newProjection = new Projection(projection.getKind(),
+                projection.hasExpression() ? (Expression) projection.getExpression().accept(this, env).first : null,
+                projection.getName());
         newProjection.setSourceLocation(projection.getSourceLocation());
         return new Pair<>(newProjection, env);
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
index 395e916..34ca727 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
@@ -123,18 +123,21 @@
 
     @Override
     public Void visit(Projection projection, Integer step) throws CompilationException {
-        if (projection.star()) {
-            out.print(" * ");
-            return null;
-        }
-        projection.getExpression().accept(this, step);
-        if (projection.varStar()) {
-            out.print(".* ");
-        } else {
-            String name = projection.getName();
-            if (name != null) {
-                out.print(" as " + name);
-            }
+        switch (projection.getKind()) {
+            case STAR:
+                out.print(" * ");
+                break;
+            case VAR_STAR:
+                projection.getExpression().accept(this, step);
+                out.print(".* ");
+                break;
+            case NAMED_EXPR:
+                projection.getExpression().accept(this, step);
+                String name = projection.getName();
+                if (name != null) {
+                    out.print(" as " + name);
+                }
+                break;
         }
         return null;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
index e331173..ef8b43c 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
@@ -113,7 +113,7 @@
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (!projection.star()) {
+        if (projection.hasExpression()) {
             projection.setExpression(visit(projection.getExpression(), arg));
         }
         return null;
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index e94ad6b..4b8e873 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
+++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
@@ -3055,7 +3055,7 @@
   <STRING_LITERAL>
     {
       return removeQuotesAndEscapes(token.image);
-    }
+  }
 }
 
 Triple<List<String>, Token, Token> MultipartIdentifier() throws ParseException:
@@ -4507,7 +4507,7 @@
   {
     SourceLocation sourceLoc = getSourceLocation(startToken);
     if (selectRegular == null && selectElement == null){
-        Projection projection = new Projection(null, null, true, false);
+        Projection projection = new Projection(Projection.Kind.STAR, null, null);
         projection.setSourceLocation(sourceLoc);
         List<Projection> projections = new ArrayList<Projection>();
         projections.add(projection);
@@ -4564,15 +4564,17 @@
   Expression expr = null;
   Identifier identifier = null;
   String name = null;
+  Projection.Kind kind = null;
   boolean star = false;
   boolean varStar = false;
 }
 {
   (
-    <MUL> { star = true; startSrcLoc = getSourceLocation(token); }
-    | LOOKAHEAD(3) expr = VariableRef() <DOT> <MUL> { varStar = true; }
+    <MUL> { kind = Projection.Kind.STAR; startSrcLoc = getSourceLocation(token); }
+    | LOOKAHEAD(3) expr = VariableRef() <DOT> <MUL> { kind = Projection.Kind.VAR_STAR; }
     | expr = Expression() ((<AS>)? name = Identifier())?
       {
+        kind = Projection.Kind.NAMED_EXPR;
         if (name == null) {
           String generatedColumnIdentifier = ExpressionToVariableUtil.getGeneratedIdentifier(expr, false);
           if (generatedColumnIdentifier != null) {
@@ -4582,7 +4584,7 @@
       }
   )
   {
-    Projection projection = new Projection(expr, name, star, varStar);
+    Projection projection = new Projection(kind, expr, name);
     projection.setSourceLocation(expr != null ? expr.getSourceLocation() : startSrcLoc);
     return projection;
   }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-MessageType: newchange

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Jenkins <je...@fulliautomatix.ics.uci.edu>:

Jenkins has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )

Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................


Patch Set 2: Integration-Tests+1

Integration Tests Successful

https://asterix-jenkins.ics.uci.edu/job/asterix-gerrit-integration-tests/12542/ : SUCCESS


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 2
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-CC: Anon. E. Moose #1000171
Gerrit-Comment-Date: Fri, 24 Sep 2021 00:23:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Dmitry Lychagin <dm...@couchbase.com>:

Dmitry Lychagin has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )


Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................

[NO ISSUE][COMP] Refactor Projection clause

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- Introduce Projection.Kind for projection kinds
  and use it instead of boolean flags
- OperatorExpr.opIsComparison() should return true
  for DISTINCT/NOT_DISTINCT operators
- Add missing setters to SelectClause and SelectExpression

Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
---
M asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
19 files changed, 182 insertions(+), 90 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/64/13364/1

diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
index a37a366..3deedfd 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
@@ -77,6 +77,8 @@
             case GE:
             case LT:
             case LE:
+            case DISTINCT:
+            case NOT_DISTINCT:
                 return true;
             default:
                 return false;
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
index 7f353d6..7aa3cec 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
@@ -29,16 +29,56 @@
 
 public class Projection extends AbstractClause {
 
+    public enum Kind {
+        NAMED_EXPR, // expr AS name
+        STAR, // *
+        VAR_STAR // variable.*
+    }
+
+    private Kind kind;
     private Expression expr;
     private String name;
-    private boolean star;
-    private boolean varStar;
 
-    public Projection(Expression expr, String name, boolean star, boolean varStar) {
+    public Projection(Kind kind, Expression expr, String name) {
+        validateKind(kind, expr, name);
+        this.kind = kind;
         this.expr = expr;
         this.name = name;
-        this.star = star;
-        this.varStar = varStar;
+    }
+
+    private static void validateKind(Kind kind, Expression expr, String name) {
+        switch (kind) {
+            case NAMED_EXPR:
+                if (expr == null) {
+                    throw new NullPointerException();
+                }
+                break;
+            case STAR:
+                if (expr != null || name != null) {
+                    throw new IllegalArgumentException();
+                }
+                break;
+            case VAR_STAR:
+                if (expr == null) {
+                    throw new NullPointerException();
+                }
+                if (name != null) {
+                    throw new IllegalArgumentException();
+                }
+                break;
+            default:
+                throw new IllegalArgumentException();
+        }
+    }
+
+    @Deprecated
+    public Projection(Expression expr, String name, boolean star, boolean varStar) {
+        this(asKind(star, varStar), expr, name);
+    }
+
+    @Deprecated
+    private static Kind asKind(boolean star, boolean varStar) {
+        return star ? Kind.STAR : varStar ? Kind.VAR_STAR : Kind.NAMED_EXPR;
     }
 
     @Override
@@ -59,6 +99,10 @@
         this.expr = expr;
     }
 
+    public boolean hasExpression() {
+        return expr != null;
+    }
+
     public String getName() {
         return name;
     }
@@ -71,22 +115,41 @@
         return name != null;
     }
 
-    public boolean star() {
-        return star;
+    public Kind getKind() {
+        return kind;
     }
 
+    public void setKind(Kind kind) {
+        this.kind = kind;
+    }
+
+    @Deprecated
+    public boolean star() {
+        return kind == Kind.STAR;
+    }
+
+    @Deprecated
     public boolean varStar() {
-        return varStar;
+        return kind == Kind.VAR_STAR;
     }
 
     @Override
     public String toString() {
-        return star ? "*" : (String.valueOf(expr) + (varStar ? ".*" : (hasName() ? " as " + getName() : "")));
+        switch (kind) {
+            case NAMED_EXPR:
+                return expr + (hasName() ? " as " + getName() : "");
+            case STAR:
+                return "*";
+            case VAR_STAR:
+                return expr + ".*";
+            default:
+                throw new IllegalStateException();
+        }
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(expr, varStar, name, star);
+        return Objects.hash(kind, expr, name);
     }
 
     @Override
@@ -98,7 +161,6 @@
             return false;
         }
         Projection target = (Projection) object;
-        return Objects.equals(expr, target.expr) && Objects.equals(name, target.name) && varStar == target.varStar
-                && star == target.star;
+        return kind == target.kind && Objects.equals(expr, target.expr) && Objects.equals(name, target.name);
     }
 }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
index 3b550a3..ea6bac9 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
@@ -48,6 +48,16 @@
         return ClauseType.SELECT_CLAUSE;
     }
 
+    public void setSelectElement(SelectElement selectElement) {
+        this.selectElement = selectElement;
+        this.selectRegular = null;
+    }
+
+    public void setSelectRegular(SelectRegular selectRegular) {
+        this.selectRegular = selectRegular;
+        this.selectElement = null;
+    }
+
     public SelectElement getSelectElement() {
         return selectElement;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
index 62b8734..2a3ec4e 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
@@ -68,10 +68,18 @@
         return selectSetOperation;
     }
 
+    public void setOrderbyClause(OrderbyClause orderbyClause) {
+        this.orderbyClause = orderbyClause;
+    }
+
     public OrderbyClause getOrderbyClause() {
         return orderbyClause;
     }
 
+    public void setLimitClause(LimitClause limitClause) {
+        this.limitClause = limitClause;
+    }
+
     public LimitClause getLimitClause() {
         return limitClause;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
index 3067641..e488e9d 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
@@ -67,7 +67,7 @@
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (!projection.star() && !projection.varStar() && projection.getName() == null) {
+        if (projection.getKind() == Projection.Kind.NAMED_EXPR && !projection.hasName()) {
             projection.setName(SqlppVariableUtil.variableNameToDisplayedFieldName(context.newVariable().getValue()));
         }
         return super.visit(projection, arg);
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
index 416f49b..53cd6de 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
@@ -151,7 +151,7 @@
     private Map<Expression, ColumnAliasBinding> mapProjections(List<Projection> projections) {
         Map<Expression, ColumnAliasBinding> exprMap = new HashMap<>();
         for (Projection projection : projections) {
-            if (!projection.star() && !projection.varStar()) {
+            if (projection.getKind() == Projection.Kind.NAMED_EXPR) {
                 String varName = SqlppVariableUtil.toInternalVariableName(projection.getName());
                 exprMap.put(new VariableExpr(new VarIdentifier(varName)), ColumnAliasBinding.of(projection));
             }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
index 8559c96..592d3f6 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
@@ -89,10 +89,7 @@
 
     @Override
     public Void visit(Projection projection, Void arg) throws CompilationException {
-        if (!projection.star()) {
-            projection.getExpression().accept(this, arg);
-        }
-        return null;
+        return projection.hasExpression() ? projection.getExpression().accept(this, arg) : null;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
index 791bfe1..9aca37a 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
@@ -177,33 +177,43 @@
         List<VariableExpr> gbyBindingVars = null, postGbyBindingVars = null;
         for (VariableExpr freeVarPostSetOp : freeVarsPostSetOp) {
             String projectionName = null;
-            for (int i = projectionList.size() - 1; i >= 0; i--) {
+            inner_loop: for (int i = projectionList.size() - 1; i >= 0; i--) {
                 Projection projection = projectionList.get(i);
-                if (projection.varStar()) {
-                    throw new CompilationException(ErrorCode.UNSUPPORTED_GBY_OBY_SELECT_COMBO,
-                            selectBlock.getSourceLocation());
-                } else if (projection.star()) {
-                    if (gbyBindingVars == null) {
-                        gbyBindingVars = SqlppVariableUtil.getBindingVariables(selectBlock.getGroupbyClause());
-                    }
-                    if (postGbyBindingVars == null) {
-                        postGbyBindingVars = selectBlock.hasLetHavingClausesAfterGroupby()
-                                ? SqlppVariableUtil.getLetBindingVariables(selectBlock.getLetHavingListAfterGroupby())
-                                : Collections.emptyList();
-                    }
-                    if (gbyBindingVars.contains(freeVarPostSetOp) || postGbyBindingVars.contains(freeVarPostSetOp)) {
-                        projectionName = SqlppVariableUtil
-                                .variableNameToDisplayedFieldName(freeVarPostSetOp.getVar().getValue());
+                switch (projection.getKind()) {
+                    case VAR_STAR:
+                        throw new CompilationException(ErrorCode.UNSUPPORTED_GBY_OBY_SELECT_COMBO,
+                                selectBlock.getSourceLocation());
+                    case STAR:
+                        if (gbyBindingVars == null) {
+                            gbyBindingVars = SqlppVariableUtil.getBindingVariables(selectBlock.getGroupbyClause());
+                        }
+                        if (postGbyBindingVars == null) {
+                            postGbyBindingVars =
+                                    selectBlock.hasLetHavingClausesAfterGroupby()
+                                            ? SqlppVariableUtil
+                                                    .getLetBindingVariables(selectBlock.getLetHavingListAfterGroupby())
+                                            : Collections.emptyList();
+                        }
+                        if (gbyBindingVars.contains(freeVarPostSetOp)
+                                || postGbyBindingVars.contains(freeVarPostSetOp)) {
+                            projectionName = SqlppVariableUtil
+                                    .variableNameToDisplayedFieldName(freeVarPostSetOp.getVar().getValue());
+                            break;
+                        }
                         break;
-                    }
-                } else if (projection.hasName()) {
-                    if (projection.getExpression().equals(freeVarPostSetOp)) {
-                        projectionName = projection.getName();
+                    case NAMED_EXPR:
+                        if (!projection.hasName()) {
+                            throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE,
+                                    selectBlock.getSourceLocation(), "");
+                        }
+                        if (projection.getExpression().equals(freeVarPostSetOp)) {
+                            projectionName = projection.getName();
+                            break inner_loop;
+                        }
                         break;
-                    }
-                } else {
-                    throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, selectBlock.getSourceLocation(),
-                            "");
+                    default:
+                        throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE,
+                                selectBlock.getSourceLocation(), "");
                 }
             }
 
@@ -396,7 +406,8 @@
         }
         SelectElement selectElement = selectClause.getSelectElement();
         List<Projection> projectionList = new ArrayList<>(1);
-        projectionList.add(new Projection(selectElement.getExpression(), mainProjectionName, false, false));
+        projectionList
+                .add(new Projection(Projection.Kind.NAMED_EXPR, selectElement.getExpression(), mainProjectionName));
         SelectRegular newSelectRegular = new SelectRegular(projectionList);
         newSelectRegular.setSourceLocation(selectElement.getSourceLocation());
         SelectClause newSelectClause = new SelectClause(null, newSelectRegular, selectClause.distinct());
@@ -412,8 +423,8 @@
         }
         SelectRegular selectRegular = selectClause.getSelectRegular();
         for (Map.Entry<VariableExpr, String> me : projections.entrySet()) {
-            Projection newProjection =
-                    new Projection((VariableExpr) SqlppRewriteUtil.deepCopy(me.getKey()), me.getValue(), false, false);
+            Projection newProjection = new Projection(Projection.Kind.NAMED_EXPR,
+                    (VariableExpr) SqlppRewriteUtil.deepCopy(me.getKey()), me.getValue());
             selectRegular.getProjections().add(newProjection);
         }
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
index 56fdd19..9f93363 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
@@ -119,7 +119,7 @@
 
     @Override
     public Boolean visit(Projection projection, Void arg) throws CompilationException {
-        if (projection.star()) {
+        if (!projection.hasExpression()) {
             return false;
         }
         Pair<Boolean, Expression> p = inlineUdfsAndViewsInExpr(projection.getExpression());
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
index a8db06f..8a3b913 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
@@ -370,7 +370,8 @@
     }
 
     private Projection createProjection(VariableExpr var, String fieldName, SourceLocation sourceLoc) {
-        Projection projection = new Projection(newVariableExpr(var.getVar(), null), fieldName, false, false);
+        Projection projection =
+                new Projection(Projection.Kind.NAMED_EXPR, newVariableExpr(var.getVar(), null), fieldName);
         projection.setSourceLocation(sourceLoc);
         return projection;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
index 027560c..3192541 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
@@ -240,10 +240,7 @@
 
     @Override
     public Boolean visit(Projection projection, ILangExpression parentSelectBlock) throws CompilationException {
-        if (projection.star()) {
-            return false;
-        }
-        return projection.getExpression().accept(this, parentSelectBlock);
+        return projection.hasExpression() ? projection.getExpression().accept(this, parentSelectBlock) : false;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
index bc31e62..a903279 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
@@ -107,10 +107,7 @@
 
     @Override
     public Boolean visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (projection.star()) {
-            return false;
-        }
-        return visit(projection.getExpression(), arg);
+        return projection.hasExpression() && visit(projection.getExpression(), arg);
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
index ec6de6e..283b416 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
@@ -143,9 +143,9 @@
 
     @Override
     public Projection visit(Projection projection, Void arg) throws CompilationException {
-        Projection copy =
-                new Projection(projection.star() ? null : (Expression) projection.getExpression().accept(this, arg),
-                        projection.getName(), projection.star(), projection.varStar());
+        Projection copy = new Projection(projection.getKind(),
+                projection.hasExpression() ? (Expression) projection.getExpression().accept(this, arg) : null,
+                projection.getName());
         copy.setSourceLocation(projection.getSourceLocation());
         return copy;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
index 0c14688..77b9991 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
@@ -146,10 +146,7 @@
 
     @Override
     public Void visit(Projection projection, Collection<VariableExpr> freeVars) throws CompilationException {
-        if (!projection.star()) {
-            projection.getExpression().accept(this, freeVars);
-        }
-        return null;
+        return projection.hasExpression() ? projection.getExpression().accept(this, freeVars) : null;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
index 90e7448..27c3952 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
@@ -131,11 +131,18 @@
 
     @Override
     public Void visit(Projection projection, Integer step) throws CompilationException {
-        if (projection.star()) {
-            out.println(skip(step) + "*");
-        } else {
-            projection.getExpression().accept(this, step);
-            out.println(skip(step) + (projection.varStar() ? ".*" : projection.getName()));
+        switch (projection.getKind()) {
+            case STAR:
+                out.println(skip(step) + "*");
+                break;
+            case VAR_STAR:
+                projection.getExpression().accept(this, step);
+                out.println(skip(step) + ".*");
+                break;
+            case NAMED_EXPR:
+                projection.getExpression().accept(this, step);
+                out.println(skip(step) + projection.getName());
+                break;
         }
         return null;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
index bd64314..e664d49 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
@@ -202,11 +202,9 @@
     @Override
     public Pair<ILangExpression, VariableSubstitutionEnvironment> visit(Projection projection,
             VariableSubstitutionEnvironment env) throws CompilationException {
-        if (projection.star()) {
-            return new Pair<>(projection, env);
-        }
-        Projection newProjection = new Projection((Expression) projection.getExpression().accept(this, env).first,
-                projection.getName(), projection.star(), projection.varStar());
+        Projection newProjection = new Projection(projection.getKind(),
+                projection.hasExpression() ? (Expression) projection.getExpression().accept(this, env).first : null,
+                projection.getName());
         newProjection.setSourceLocation(projection.getSourceLocation());
         return new Pair<>(newProjection, env);
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
index 395e916..34ca727 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
@@ -123,18 +123,21 @@
 
     @Override
     public Void visit(Projection projection, Integer step) throws CompilationException {
-        if (projection.star()) {
-            out.print(" * ");
-            return null;
-        }
-        projection.getExpression().accept(this, step);
-        if (projection.varStar()) {
-            out.print(".* ");
-        } else {
-            String name = projection.getName();
-            if (name != null) {
-                out.print(" as " + name);
-            }
+        switch (projection.getKind()) {
+            case STAR:
+                out.print(" * ");
+                break;
+            case VAR_STAR:
+                projection.getExpression().accept(this, step);
+                out.print(".* ");
+                break;
+            case NAMED_EXPR:
+                projection.getExpression().accept(this, step);
+                String name = projection.getName();
+                if (name != null) {
+                    out.print(" as " + name);
+                }
+                break;
         }
         return null;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
index e331173..ef8b43c 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
@@ -113,7 +113,7 @@
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (!projection.star()) {
+        if (projection.hasExpression()) {
             projection.setExpression(visit(projection.getExpression(), arg));
         }
         return null;
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index e94ad6b..4b8e873 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
+++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
@@ -3055,7 +3055,7 @@
   <STRING_LITERAL>
     {
       return removeQuotesAndEscapes(token.image);
-    }
+  }
 }
 
 Triple<List<String>, Token, Token> MultipartIdentifier() throws ParseException:
@@ -4507,7 +4507,7 @@
   {
     SourceLocation sourceLoc = getSourceLocation(startToken);
     if (selectRegular == null && selectElement == null){
-        Projection projection = new Projection(null, null, true, false);
+        Projection projection = new Projection(Projection.Kind.STAR, null, null);
         projection.setSourceLocation(sourceLoc);
         List<Projection> projections = new ArrayList<Projection>();
         projections.add(projection);
@@ -4564,15 +4564,17 @@
   Expression expr = null;
   Identifier identifier = null;
   String name = null;
+  Projection.Kind kind = null;
   boolean star = false;
   boolean varStar = false;
 }
 {
   (
-    <MUL> { star = true; startSrcLoc = getSourceLocation(token); }
-    | LOOKAHEAD(3) expr = VariableRef() <DOT> <MUL> { varStar = true; }
+    <MUL> { kind = Projection.Kind.STAR; startSrcLoc = getSourceLocation(token); }
+    | LOOKAHEAD(3) expr = VariableRef() <DOT> <MUL> { kind = Projection.Kind.VAR_STAR; }
     | expr = Expression() ((<AS>)? name = Identifier())?
       {
+        kind = Projection.Kind.NAMED_EXPR;
         if (name == null) {
           String generatedColumnIdentifier = ExpressionToVariableUtil.getGeneratedIdentifier(expr, false);
           if (generatedColumnIdentifier != null) {
@@ -4582,7 +4584,7 @@
       }
   )
   {
-    Projection projection = new Projection(expr, name, star, varStar);
+    Projection projection = new Projection(kind, expr, name);
     projection.setSourceLocation(expr != null ? expr.getSourceLocation() : startSrcLoc);
     return projection;
   }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 1
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-MessageType: newchange

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Dmitry Lychagin <dm...@couchbase.com>:

Dmitry Lychagin has uploaded a new patch set (#2). ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )

Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................

[NO ISSUE][COMP] Refactor Projection clause

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- Introduce Projection.Kind for projection kinds
  and use it instead of boolean flags
- OperatorExpr.opIsComparison() should return true
  for DISTINCT/NOT_DISTINCT operators
- Add missing setters to SelectClause and SelectExpression

Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
---
M asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
19 files changed, 181 insertions(+), 89 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/64/13364/2
-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 2
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-CC: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-MessageType: newpatchset

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Dmitry Lychagin <dm...@couchbase.com>:

Dmitry Lychagin has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )

Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................

[NO ISSUE][COMP] Refactor Projection clause

- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- Introduce Projection.Kind for projection kinds
  and use it instead of boolean flags
- OperatorExpr.opIsComparison() should return true
  for DISTINCT/NOT_DISTINCT operators
- Add missing setters to SelectClause and SelectExpression

Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
M asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
M asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
19 files changed, 181 insertions(+), 89 deletions(-)

Approvals:
  Dmitry Lychagin: Looks good to me, but someone else must approve
  Ali Alsuliman: Looks good to me, approved
  Jenkins: Verified; Verified

Objections:
  Anon. E. Moose #1000171: Violations found



diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
index a37a366..3deedfd 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/expression/OperatorExpr.java
@@ -77,6 +77,8 @@
             case GE:
             case LT:
             case LE:
+            case DISTINCT:
+            case NOT_DISTINCT:
                 return true;
             default:
                 return false;
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
index 7f353d6..7aa3cec 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/Projection.java
@@ -29,16 +29,56 @@
 
 public class Projection extends AbstractClause {
 
+    public enum Kind {
+        NAMED_EXPR, // expr AS name
+        STAR, // *
+        VAR_STAR // variable.*
+    }
+
+    private Kind kind;
     private Expression expr;
     private String name;
-    private boolean star;
-    private boolean varStar;
 
-    public Projection(Expression expr, String name, boolean star, boolean varStar) {
+    public Projection(Kind kind, Expression expr, String name) {
+        validateKind(kind, expr, name);
+        this.kind = kind;
         this.expr = expr;
         this.name = name;
-        this.star = star;
-        this.varStar = varStar;
+    }
+
+    private static void validateKind(Kind kind, Expression expr, String name) {
+        switch (kind) {
+            case NAMED_EXPR:
+                if (expr == null) {
+                    throw new NullPointerException();
+                }
+                break;
+            case STAR:
+                if (expr != null || name != null) {
+                    throw new IllegalArgumentException();
+                }
+                break;
+            case VAR_STAR:
+                if (expr == null) {
+                    throw new NullPointerException();
+                }
+                if (name != null) {
+                    throw new IllegalArgumentException();
+                }
+                break;
+            default:
+                throw new IllegalArgumentException();
+        }
+    }
+
+    @Deprecated
+    public Projection(Expression expr, String name, boolean star, boolean varStar) {
+        this(asKind(star, varStar), expr, name);
+    }
+
+    @Deprecated
+    private static Kind asKind(boolean star, boolean varStar) {
+        return star ? Kind.STAR : varStar ? Kind.VAR_STAR : Kind.NAMED_EXPR;
     }
 
     @Override
@@ -59,6 +99,10 @@
         this.expr = expr;
     }
 
+    public boolean hasExpression() {
+        return expr != null;
+    }
+
     public String getName() {
         return name;
     }
@@ -71,22 +115,41 @@
         return name != null;
     }
 
-    public boolean star() {
-        return star;
+    public Kind getKind() {
+        return kind;
     }
 
+    public void setKind(Kind kind) {
+        this.kind = kind;
+    }
+
+    @Deprecated
+    public boolean star() {
+        return kind == Kind.STAR;
+    }
+
+    @Deprecated
     public boolean varStar() {
-        return varStar;
+        return kind == Kind.VAR_STAR;
     }
 
     @Override
     public String toString() {
-        return star ? "*" : (String.valueOf(expr) + (varStar ? ".*" : (hasName() ? " as " + getName() : "")));
+        switch (kind) {
+            case NAMED_EXPR:
+                return expr + (hasName() ? " as " + getName() : "");
+            case STAR:
+                return "*";
+            case VAR_STAR:
+                return expr + ".*";
+            default:
+                throw new IllegalStateException();
+        }
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(expr, varStar, name, star);
+        return Objects.hash(kind, expr, name);
     }
 
     @Override
@@ -98,7 +161,6 @@
             return false;
         }
         Projection target = (Projection) object;
-        return Objects.equals(expr, target.expr) && Objects.equals(name, target.name) && varStar == target.varStar
-                && star == target.star;
+        return kind == target.kind && Objects.equals(expr, target.expr) && Objects.equals(name, target.name);
     }
 }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
index 3b550a3..ea6bac9 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/clause/SelectClause.java
@@ -48,6 +48,16 @@
         return ClauseType.SELECT_CLAUSE;
     }
 
+    public void setSelectElement(SelectElement selectElement) {
+        this.selectElement = selectElement;
+        this.selectRegular = null;
+    }
+
+    public void setSelectRegular(SelectRegular selectRegular) {
+        this.selectRegular = selectRegular;
+        this.selectElement = null;
+    }
+
     public SelectElement getSelectElement() {
         return selectElement;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
index 62b8734..2a3ec4e 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/expression/SelectExpression.java
@@ -68,10 +68,18 @@
         return selectSetOperation;
     }
 
+    public void setOrderbyClause(OrderbyClause orderbyClause) {
+        this.orderbyClause = orderbyClause;
+    }
+
     public OrderbyClause getOrderbyClause() {
         return orderbyClause;
     }
 
+    public void setLimitClause(LimitClause limitClause) {
+        this.limitClause = limitClause;
+    }
+
     public LimitClause getLimitClause() {
         return limitClause;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
index 3067641..e488e9d 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/GenerateColumnNameVisitor.java
@@ -67,7 +67,7 @@
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (!projection.star() && !projection.varStar() && projection.getName() == null) {
+        if (projection.getKind() == Projection.Kind.NAMED_EXPR && !projection.hasName()) {
             projection.setName(SqlppVariableUtil.variableNameToDisplayedFieldName(context.newVariable().getValue()));
         }
         return super.visit(projection, arg);
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
index 416f49b..53cd6de 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
@@ -151,7 +151,7 @@
     private Map<Expression, ColumnAliasBinding> mapProjections(List<Projection> projections) {
         Map<Expression, ColumnAliasBinding> exprMap = new HashMap<>();
         for (Projection projection : projections) {
-            if (!projection.star() && !projection.varStar()) {
+            if (projection.getKind() == Projection.Kind.NAMED_EXPR) {
                 String varName = SqlppVariableUtil.toInternalVariableName(projection.getName());
                 exprMap.put(new VariableExpr(new VarIdentifier(varName)), ColumnAliasBinding.of(projection));
             }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
index 8559c96..592d3f6 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGatherFunctionCallsVisitor.java
@@ -89,10 +89,7 @@
 
     @Override
     public Void visit(Projection projection, Void arg) throws CompilationException {
-        if (!projection.star()) {
-            projection.getExpression().accept(this, arg);
-        }
-        return null;
+        return projection.hasExpression() ? projection.getExpression().accept(this, arg) : null;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
index 791bfe1..9aca37a 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupingSetsVisitor.java
@@ -177,33 +177,43 @@
         List<VariableExpr> gbyBindingVars = null, postGbyBindingVars = null;
         for (VariableExpr freeVarPostSetOp : freeVarsPostSetOp) {
             String projectionName = null;
-            for (int i = projectionList.size() - 1; i >= 0; i--) {
+            inner_loop: for (int i = projectionList.size() - 1; i >= 0; i--) {
                 Projection projection = projectionList.get(i);
-                if (projection.varStar()) {
-                    throw new CompilationException(ErrorCode.UNSUPPORTED_GBY_OBY_SELECT_COMBO,
-                            selectBlock.getSourceLocation());
-                } else if (projection.star()) {
-                    if (gbyBindingVars == null) {
-                        gbyBindingVars = SqlppVariableUtil.getBindingVariables(selectBlock.getGroupbyClause());
-                    }
-                    if (postGbyBindingVars == null) {
-                        postGbyBindingVars = selectBlock.hasLetHavingClausesAfterGroupby()
-                                ? SqlppVariableUtil.getLetBindingVariables(selectBlock.getLetHavingListAfterGroupby())
-                                : Collections.emptyList();
-                    }
-                    if (gbyBindingVars.contains(freeVarPostSetOp) || postGbyBindingVars.contains(freeVarPostSetOp)) {
-                        projectionName = SqlppVariableUtil
-                                .variableNameToDisplayedFieldName(freeVarPostSetOp.getVar().getValue());
+                switch (projection.getKind()) {
+                    case VAR_STAR:
+                        throw new CompilationException(ErrorCode.UNSUPPORTED_GBY_OBY_SELECT_COMBO,
+                                selectBlock.getSourceLocation());
+                    case STAR:
+                        if (gbyBindingVars == null) {
+                            gbyBindingVars = SqlppVariableUtil.getBindingVariables(selectBlock.getGroupbyClause());
+                        }
+                        if (postGbyBindingVars == null) {
+                            postGbyBindingVars =
+                                    selectBlock.hasLetHavingClausesAfterGroupby()
+                                            ? SqlppVariableUtil
+                                                    .getLetBindingVariables(selectBlock.getLetHavingListAfterGroupby())
+                                            : Collections.emptyList();
+                        }
+                        if (gbyBindingVars.contains(freeVarPostSetOp)
+                                || postGbyBindingVars.contains(freeVarPostSetOp)) {
+                            projectionName = SqlppVariableUtil
+                                    .variableNameToDisplayedFieldName(freeVarPostSetOp.getVar().getValue());
+                            break;
+                        }
                         break;
-                    }
-                } else if (projection.hasName()) {
-                    if (projection.getExpression().equals(freeVarPostSetOp)) {
-                        projectionName = projection.getName();
+                    case NAMED_EXPR:
+                        if (!projection.hasName()) {
+                            throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE,
+                                    selectBlock.getSourceLocation(), "");
+                        }
+                        if (projection.getExpression().equals(freeVarPostSetOp)) {
+                            projectionName = projection.getName();
+                            break inner_loop;
+                        }
                         break;
-                    }
-                } else {
-                    throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, selectBlock.getSourceLocation(),
-                            "");
+                    default:
+                        throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE,
+                                selectBlock.getSourceLocation(), "");
                 }
             }
 
@@ -396,7 +406,8 @@
         }
         SelectElement selectElement = selectClause.getSelectElement();
         List<Projection> projectionList = new ArrayList<>(1);
-        projectionList.add(new Projection(selectElement.getExpression(), mainProjectionName, false, false));
+        projectionList
+                .add(new Projection(Projection.Kind.NAMED_EXPR, selectElement.getExpression(), mainProjectionName));
         SelectRegular newSelectRegular = new SelectRegular(projectionList);
         newSelectRegular.setSourceLocation(selectElement.getSourceLocation());
         SelectClause newSelectClause = new SelectClause(null, newSelectRegular, selectClause.distinct());
@@ -412,8 +423,8 @@
         }
         SelectRegular selectRegular = selectClause.getSelectRegular();
         for (Map.Entry<VariableExpr, String> me : projections.entrySet()) {
-            Projection newProjection =
-                    new Projection((VariableExpr) SqlppRewriteUtil.deepCopy(me.getKey()), me.getValue(), false, false);
+            Projection newProjection = new Projection(Projection.Kind.NAMED_EXPR,
+                    (VariableExpr) SqlppRewriteUtil.deepCopy(me.getKey()), me.getValue());
             selectRegular.getProjections().add(newProjection);
         }
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
index 56fdd19..9f93363 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
@@ -119,7 +119,7 @@
 
     @Override
     public Boolean visit(Projection projection, Void arg) throws CompilationException {
-        if (projection.star()) {
+        if (!projection.hasExpression()) {
             return false;
         }
         Pair<Boolean, Expression> p = inlineUdfsAndViewsInExpr(projection.getExpression());
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
index a8db06f..8a3b913 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppRightJoinRewriteVisitor.java
@@ -370,7 +370,8 @@
     }
 
     private Projection createProjection(VariableExpr var, String fieldName, SourceLocation sourceLoc) {
-        Projection projection = new Projection(newVariableExpr(var.getVar(), null), fieldName, false, false);
+        Projection projection =
+                new Projection(Projection.Kind.NAMED_EXPR, newVariableExpr(var.getVar(), null), fieldName);
         projection.setSourceLocation(sourceLoc);
         return projection;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
index 027560c..3192541 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSql92AggregateVisitor.java
@@ -240,10 +240,7 @@
 
     @Override
     public Boolean visit(Projection projection, ILangExpression parentSelectBlock) throws CompilationException {
-        if (projection.star()) {
-            return false;
-        }
-        return projection.getExpression().accept(this, parentSelectBlock);
+        return projection.hasExpression() ? projection.getExpression().accept(this, parentSelectBlock) : false;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
index bc31e62..a903279 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/CheckSubqueryVisitor.java
@@ -107,10 +107,7 @@
 
     @Override
     public Boolean visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (projection.star()) {
-            return false;
-        }
-        return visit(projection.getExpression(), arg);
+        return projection.hasExpression() && visit(projection.getExpression(), arg);
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
index ec6de6e..283b416 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
@@ -143,9 +143,9 @@
 
     @Override
     public Projection visit(Projection projection, Void arg) throws CompilationException {
-        Projection copy =
-                new Projection(projection.star() ? null : (Expression) projection.getExpression().accept(this, arg),
-                        projection.getName(), projection.star(), projection.varStar());
+        Projection copy = new Projection(projection.getKind(),
+                projection.hasExpression() ? (Expression) projection.getExpression().accept(this, arg) : null,
+                projection.getName());
         copy.setSourceLocation(projection.getSourceLocation());
         return copy;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
index 0c14688..77b9991 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
@@ -146,10 +146,7 @@
 
     @Override
     public Void visit(Projection projection, Collection<VariableExpr> freeVars) throws CompilationException {
-        if (!projection.star()) {
-            projection.getExpression().accept(this, freeVars);
-        }
-        return null;
+        return projection.hasExpression() ? projection.getExpression().accept(this, freeVars) : null;
     }
 
     @Override
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
index 90e7448..27c3952 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppAstPrintVisitor.java
@@ -131,11 +131,18 @@
 
     @Override
     public Void visit(Projection projection, Integer step) throws CompilationException {
-        if (projection.star()) {
-            out.println(skip(step) + "*");
-        } else {
-            projection.getExpression().accept(this, step);
-            out.println(skip(step) + (projection.varStar() ? ".*" : projection.getName()));
+        switch (projection.getKind()) {
+            case STAR:
+                out.println(skip(step) + "*");
+                break;
+            case VAR_STAR:
+                projection.getExpression().accept(this, step);
+                out.println(skip(step) + ".*");
+                break;
+            case NAMED_EXPR:
+                projection.getExpression().accept(this, step);
+                out.println(skip(step) + projection.getName());
+                break;
         }
         return null;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
index bd64314..e664d49 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
@@ -202,11 +202,9 @@
     @Override
     public Pair<ILangExpression, VariableSubstitutionEnvironment> visit(Projection projection,
             VariableSubstitutionEnvironment env) throws CompilationException {
-        if (projection.star()) {
-            return new Pair<>(projection, env);
-        }
-        Projection newProjection = new Projection((Expression) projection.getExpression().accept(this, env).first,
-                projection.getName(), projection.star(), projection.varStar());
+        Projection newProjection = new Projection(projection.getKind(),
+                projection.hasExpression() ? (Expression) projection.getExpression().accept(this, env).first : null,
+                projection.getName());
         newProjection.setSourceLocation(projection.getSourceLocation());
         return new Pair<>(newProjection, env);
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
index 395e916..34ca727 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppFormatPrintVisitor.java
@@ -123,18 +123,21 @@
 
     @Override
     public Void visit(Projection projection, Integer step) throws CompilationException {
-        if (projection.star()) {
-            out.print(" * ");
-            return null;
-        }
-        projection.getExpression().accept(this, step);
-        if (projection.varStar()) {
-            out.print(".* ");
-        } else {
-            String name = projection.getName();
-            if (name != null) {
-                out.print(" as " + name);
-            }
+        switch (projection.getKind()) {
+            case STAR:
+                out.print(" * ");
+                break;
+            case VAR_STAR:
+                projection.getExpression().accept(this, step);
+                out.print(".* ");
+                break;
+            case NAMED_EXPR:
+                projection.getExpression().accept(this, step);
+                String name = projection.getName();
+                if (name != null) {
+                    out.print(" as " + name);
+                }
+                break;
         }
         return null;
     }
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
index e331173..ef8b43c 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
@@ -113,7 +113,7 @@
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws CompilationException {
-        if (!projection.star()) {
+        if (projection.hasExpression()) {
             projection.setExpression(visit(projection.getExpression(), arg));
         }
         return null;
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index e94ad6b..735cae5 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
+++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
@@ -4507,7 +4507,7 @@
   {
     SourceLocation sourceLoc = getSourceLocation(startToken);
     if (selectRegular == null && selectElement == null){
-        Projection projection = new Projection(null, null, true, false);
+        Projection projection = new Projection(Projection.Kind.STAR, null, null);
         projection.setSourceLocation(sourceLoc);
         List<Projection> projections = new ArrayList<Projection>();
         projections.add(projection);
@@ -4564,15 +4564,17 @@
   Expression expr = null;
   Identifier identifier = null;
   String name = null;
+  Projection.Kind kind = null;
   boolean star = false;
   boolean varStar = false;
 }
 {
   (
-    <MUL> { star = true; startSrcLoc = getSourceLocation(token); }
-    | LOOKAHEAD(3) expr = VariableRef() <DOT> <MUL> { varStar = true; }
+    <MUL> { kind = Projection.Kind.STAR; startSrcLoc = getSourceLocation(token); }
+    | LOOKAHEAD(3) expr = VariableRef() <DOT> <MUL> { kind = Projection.Kind.VAR_STAR; }
     | expr = Expression() ((<AS>)? name = Identifier())?
       {
+        kind = Projection.Kind.NAMED_EXPR;
         if (name == null) {
           String generatedColumnIdentifier = ExpressionToVariableUtil.getGeneratedIdentifier(expr, false);
           if (generatedColumnIdentifier != null) {
@@ -4582,7 +4584,7 @@
       }
   )
   {
-    Projection projection = new Projection(expr, name, star, varStar);
+    Projection projection = new Projection(kind, expr, name);
     projection.setSourceLocation(expr != null ? expr.getSourceLocation() : startSrcLoc);
     return projection;
   }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 3
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-MessageType: merged

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
From Dmitry Lychagin <dm...@couchbase.com>:

Dmitry Lychagin has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )

Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................


Patch Set 2: Code-Review+1


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 2
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Comment-Date: Fri, 24 Sep 2021 03:09:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in asterixdb[master]: [NO ISSUE][COMP] Refactor Projection clause

Posted by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>.
Anon. E. Moose #1000171 has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364 )

Change subject: [NO ISSUE][COMP] Refactor Projection clause
......................................................................


Patch Set 2: Contrib-2

Analytics Compatibility Tests Failed
https://cbjenkins.page.link/VkqTikmk15fgkyFi6 : UNSTABLE


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/13364
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: I2dd25b4118251c1bf1edf0906a358500c3ce89fc
Gerrit-Change-Number: 13364
Gerrit-PatchSet: 2
Gerrit-Owner: Dmitry Lychagin <dm...@couchbase.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Comment-Date: Fri, 24 Sep 2021 02:35:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment