asterixdb-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From buyin...@apache.org
Subject [3/8] asterixdb git commit: Clean up GROUP BY and WITH clause.
Date Mon, 15 Aug 2016 15:45:34 GMT
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/CloneAndSubstituteVariablesVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/CloneAndSubstituteVariablesVisitor.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/CloneAndSubstituteVariablesVisitor.java
index 9ffcf8d..3fdb141 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/CloneAndSubstituteVariablesVisitor.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/CloneAndSubstituteVariablesVisitor.java
@@ -19,8 +19,10 @@
 package org.apache.asterix.lang.common.visitor;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.lang.common.base.Expression;
@@ -88,15 +90,12 @@ public class CloneAndSubstituteVariablesVisitor extends
         if (gc.hasGroupVar()) {
             newGroupVar = generateNewVariable(context, gc.getGroupVar());
         }
-        List<VariableExpr> wList = new LinkedList<>();
-        if (gc.hasWithList()) {
-            for (VariableExpr w : gc.getWithVarList()) {
-                VarIdentifier newVar = context.getRewrittenVar(w.getVar().getId());
-                if (newVar == null) {
-                    throw new AsterixException("Could not find a rewritten variable identifier for " + w);
-                }
-                VariableExpr newWithVar = new VariableExpr(newVar);
-                wList.add(newWithVar);
+        Map<Expression, VariableExpr> newWithMap = new HashMap<>();
+        if (gc.hasWithMap()) {
+            for (Entry<Expression, VariableExpr> entry : gc.getWithVarMap().entrySet()) {
+                Expression newKeyVar = (Expression) entry.getKey().accept(this, env).first;
+                VariableExpr newValueVar = generateNewVariable(context, entry.getValue());
+                newWithMap.put(newKeyVar, newValueVar);
             }
         }
         List<Pair<Expression, Identifier>> newGroupFieldList = new ArrayList<>();
@@ -106,7 +105,7 @@ public class CloneAndSubstituteVariablesVisitor extends
                 newGroupFieldList.add(new Pair<Expression, Identifier>(newExpr, varId.second));
             }
         }
-        GroupbyClause newGroup = new GroupbyClause(newGbyList, newDecorList, wList, newGroupVar, newGroupFieldList,
+        GroupbyClause newGroup = new GroupbyClause(newGbyList, newDecorList, newWithMap, newGroupVar, newGroupFieldList,
                 gc.hasHashGroupByHint(), gc.isGroupAll());
         return new Pair<>(newGroup, newSubs);
     }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/FormatPrintVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/FormatPrintVisitor.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/FormatPrintVisitor.java
index 54795bc..5feb61b 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/FormatPrintVisitor.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/FormatPrintVisitor.java
@@ -25,6 +25,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import org.apache.asterix.common.config.DatasetConfig.DatasetType;
@@ -327,9 +328,23 @@ public class FormatPrintVisitor implements ILangVisitor<Void, Integer> {
             out.print(" decor ");
             printDelimitedGbyExpressions(gc.getDecorPairList(), step + 2);
         }
-        if (gc.hasWithList()) {
+        if (gc.hasWithMap()) {
             out.print(" with ");
-            this.printDelimitedExpressions(gc.getWithVarList(), COMMA, step + 2);
+            Map<Expression, VariableExpr> withVarMap = gc.getWithVarMap();
+            int index = 0;
+            int size = withVarMap.size();
+            for (Entry<Expression, VariableExpr> entry : withVarMap.entrySet()) {
+                Expression key = entry.getKey();
+                VariableExpr value = entry.getValue();
+                key.accept(this, step + 2);
+                if (!key.equals(value)) {
+                    out.print(" as ");
+                    value.accept(this, step + 2);
+                }
+                if (++index < size) {
+                    out.print(COMMA);
+                }
+            }
         }
         out.println();
         return null;

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/QueryPrintVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/QueryPrintVisitor.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/QueryPrintVisitor.java
index a32a3c8..a5c9efc 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/QueryPrintVisitor.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/QueryPrintVisitor.java
@@ -21,6 +21,7 @@ package org.apache.asterix.lang.common.visitor;
 import java.io.PrintWriter;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map.Entry;
 
 import org.apache.asterix.common.config.DatasetConfig.DatasetType;
 import org.apache.asterix.common.exceptions.AsterixException;
@@ -251,10 +252,16 @@ public class QueryPrintVisitor extends AbstractQueryExpressionVisitor<Void, Inte
                 pair.getExpr().accept(this, step + 1);
             }
         }
-        if (gc.hasWithList()) {
+        if (gc.hasWithMap()) {
             out.println(skip(step + 1) + "With");
-            for (VariableExpr exp : gc.getWithVarList()) {
-                exp.accept(this, step + 1);
+            for (Entry<Expression, VariableExpr> entry : gc.getWithVarMap().entrySet()) {
+                Expression key = entry.getKey();
+                VariableExpr value = entry.getValue();
+                key.accept(this, step + 1);
+                if (!key.equals(value)) {
+                    out.println(skip(step + 1) + "AS");
+                    value.accept(this, step + 1);
+                }
             }
         }
         out.println();

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/SubstituteExpressionVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/SubstituteExpressionVisitor.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/SubstituteExpressionVisitor.java
deleted file mode 100644
index 073e413..0000000
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/visitor/SubstituteExpressionVisitor.java
+++ /dev/null
@@ -1,245 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.asterix.lang.common.visitor;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.asterix.common.exceptions.AsterixException;
-import org.apache.asterix.lang.common.base.Expression;
-import org.apache.asterix.lang.common.clause.LetClause;
-import org.apache.asterix.lang.common.clause.LimitClause;
-import org.apache.asterix.lang.common.clause.OrderbyClause;
-import org.apache.asterix.lang.common.clause.WhereClause;
-import org.apache.asterix.lang.common.expression.CallExpr;
-import org.apache.asterix.lang.common.expression.FieldAccessor;
-import org.apache.asterix.lang.common.expression.FieldBinding;
-import org.apache.asterix.lang.common.expression.IfExpr;
-import org.apache.asterix.lang.common.expression.IndexAccessor;
-import org.apache.asterix.lang.common.expression.ListConstructor;
-import org.apache.asterix.lang.common.expression.LiteralExpr;
-import org.apache.asterix.lang.common.expression.OperatorExpr;
-import org.apache.asterix.lang.common.expression.QuantifiedExpression;
-import org.apache.asterix.lang.common.expression.RecordConstructor;
-import org.apache.asterix.lang.common.expression.UnaryExpr;
-import org.apache.asterix.lang.common.expression.VariableExpr;
-import org.apache.asterix.lang.common.rewrites.ExpressionSubstitutionEnvironment;
-import org.apache.asterix.lang.common.rewrites.ExpressionSubstitutionEnvironment.DeepCopier;
-import org.apache.asterix.lang.common.statement.FunctionDecl;
-import org.apache.asterix.lang.common.statement.Query;
-import org.apache.asterix.lang.common.struct.QuantifiedPair;
-import org.apache.asterix.lang.common.visitor.base.AbstractQueryExpressionVisitor;
-
-public abstract class SubstituteExpressionVisitor
-        extends AbstractQueryExpressionVisitor<Expression, ExpressionSubstitutionEnvironment> {
-    protected final DeepCopier deepCopier;
-
-    public SubstituteExpressionVisitor(DeepCopier deepCopier) {
-        this.deepCopier = deepCopier;
-    }
-
-    @Override
-    public Expression visit(LetClause lc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        // Marks the binding variable for future visiting until it exists its scope.
-        env.disableVariable(lc.getVarExpr());
-        lc.setBindingExpr(lc.getBindingExpr().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(Query q, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        q.setBody(q.getBody().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(FunctionDecl fd, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        // Do nothing for a function declaration.
-        return null;
-    }
-
-    @Override
-    public Expression visit(LiteralExpr l, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        return env.findSubstitution(l, deepCopier);
-    }
-
-    @Override
-    public Expression visit(VariableExpr v, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        return env.findSubstitution(v, deepCopier);
-    }
-
-    @Override
-    public Expression visit(ListConstructor lc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newLc = env.findSubstitution(lc, deepCopier);
-        if (newLc == lc) {
-            lc.setExprList(rewriteExpressionList(lc.getExprList(), env));
-            return lc;
-        } else {
-            return newLc.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(RecordConstructor rc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newRc = env.findSubstitution(rc, deepCopier);
-        if (newRc == rc) {
-            for (FieldBinding fb : rc.getFbList()) {
-                fb.setLeftExpr(fb.getLeftExpr().accept(this, env));
-                fb.setRightExpr(fb.getRightExpr().accept(this, env));
-            }
-            return rc;
-        } else {
-            return newRc.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(OperatorExpr operatorExpr, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newOpertorExpr = env.findSubstitution(operatorExpr, deepCopier);
-        if (newOpertorExpr == operatorExpr) {
-            operatorExpr.setExprList(rewriteExpressionList(operatorExpr.getExprList(), env));
-            return operatorExpr;
-        } else {
-            return newOpertorExpr.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(FieldAccessor fa, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newFa = env.findSubstitution(fa, deepCopier);
-        if (newFa == fa) {
-            fa.setExpr(fa.getExpr().accept(this, env));
-            return fa;
-        } else {
-            return newFa.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(IndexAccessor ia, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newIa = env.findSubstitution(ia, deepCopier);
-        if (newIa == ia) {
-            ia.setExpr(ia.getExpr().accept(this, env));
-            ia.setIndexExpr(ia.getIndexExpr().accept(this, env));
-            return ia;
-        } else {
-            return newIa.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(IfExpr ifexpr, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newIfExpr = env.findSubstitution(ifexpr, deepCopier);
-        if (newIfExpr == ifexpr) {
-            ifexpr.setCondExpr(ifexpr.getCondExpr().accept(this, env));
-            ifexpr.setThenExpr(ifexpr.getThenExpr().accept(this, env));
-            ifexpr.setElseExpr(ifexpr.getElseExpr().accept(this, env));
-            return ifexpr;
-        } else {
-            return newIfExpr.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(QuantifiedExpression qe, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newQe = env.findSubstitution(qe, deepCopier);
-        if (newQe == qe) {
-            // Rewrites the quantifier list.
-            for (QuantifiedPair pair : qe.getQuantifiedList()) {
-                pair.setExpr(pair.getExpr().accept(this, env));
-            }
-
-            // Rewrites the condition.
-            for (QuantifiedPair pair : qe.getQuantifiedList()) {
-                // Marks each binding var.
-                env.disableVariable(pair.getVarExpr());
-            }
-            qe.setSatisfiesExpr(qe.getSatisfiesExpr().accept(this, env));
-            for (QuantifiedPair pair : qe.getQuantifiedList()) {
-                // Let each binding var exit its scope.
-                env.enableVariable(pair.getVarExpr());
-            }
-            return qe;
-        } else {
-            return newQe.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(WhereClause wc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        wc.setWhereExpr(wc.getWhereExpr().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(OrderbyClause oc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        oc.setOrderbyList(rewriteExpressionList(oc.getOrderbyList(), env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(LimitClause lc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        lc.setLimitExpr(lc.getLimitExpr().accept(this, env));
-        if (lc.hasOffset()) {
-            lc.setOffset(lc.getOffset().accept(this, env));
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(UnaryExpr u, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newU = env.findSubstitution(u, deepCopier);
-        if (newU == u) {
-            u.setExpr(u.getExpr().accept(this, env));
-            return u;
-        } else {
-            return newU.accept(this, env);
-        }
-    }
-
-    @Override
-    public Expression visit(CallExpr callExpr, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newCallExpr = env.findSubstitution(callExpr, deepCopier);
-        if (newCallExpr == callExpr) {
-            callExpr.setExprList(rewriteExpressionList(callExpr.getExprList(), env));
-            return callExpr;
-        } else {
-            return newCallExpr.accept(this, env);
-        }
-    }
-
-    /**
-     * Rewrites the expression list.
-     *
-     * @param exprs,
-     *            list of expressions.
-     * @param env,
-     *            ExpressionSubstitutionEnvironment.
-     * @return a list of rewritten expressions.
-     * @throws AsterixException
-     */
-    protected List<Expression> rewriteExpressionList(List<Expression> exprs, ExpressionSubstitutionEnvironment env)
-            throws AsterixException {
-        List<Expression> newExprs = new ArrayList<>();
-        for (Expression expr : exprs) {
-            newExprs.add(env.findSubstitution(expr, deepCopier));
-        }
-        return newExprs;
-    }
-}

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppFunctionBodyRewriter.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppFunctionBodyRewriter.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppFunctionBodyRewriter.java
index 43fa4a2..7a26c6b 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppFunctionBodyRewriter.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppFunctionBodyRewriter.java
@@ -43,9 +43,6 @@ class SqlppFunctionBodyRewriter extends SqlppQueryRewriter {
         // Substitutes group-by key expressions.
         substituteGroupbyKeyExpression();
 
-        // Inlines WITH expressions.
-        inlineWithExpressions();
-
         // Rewrites SQL-92 global aggregations.
         rewriteGlobalAggregations();
 

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
index 3f7cb31..1ce5de7 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/SqlppQueryRewriter.java
@@ -107,9 +107,6 @@ class SqlppQueryRewriter implements IQueryRewriter {
         // Substitutes group-by key expressions.
         substituteGroupbyKeyExpression();
 
-        // Inlines WITH expressions.
-        inlineWithExpressions();
-
         // Rewrites SQL-92 global aggregations.
         rewriteGlobalAggregations();
 
@@ -141,6 +138,10 @@ class SqlppQueryRewriter implements IQueryRewriter {
         // Replace global variable access with the dataset function for inlined expressions.
         variableCheckAndRewrite(true);
 
+        // Inlines WITH expressions after variableCheckAndRewrite(...) so that the variable scoping for WITH
+        // expression is correct.
+        inlineWithExpressions();
+
         // Sets the var counter of the query.
         topExpr.setVarCounter(context.getVarCounter());
     }
@@ -170,7 +171,7 @@ class SqlppQueryRewriter implements IQueryRewriter {
             return;
         }
         // Inlines with expressions.
-        InlineWithExpressionVisitor inlineWithExpressionVisitor = new InlineWithExpressionVisitor();
+        InlineWithExpressionVisitor inlineWithExpressionVisitor = new InlineWithExpressionVisitor(context);
         inlineWithExpressionVisitor.visit(topExpr, null);
     }
 
@@ -189,7 +190,7 @@ class SqlppQueryRewriter implements IQueryRewriter {
         }
         // Substitute group-by key expressions that appear in the select clause.
         SubstituteGroupbyExpressionWithVariableVisitor substituteGbyExprVisitor =
-                new SubstituteGroupbyExpressionWithVariableVisitor();
+                new SubstituteGroupbyExpressionWithVariableVisitor(context);
         substituteGbyExprVisitor.visit(topExpr, null);
     }
 
@@ -216,8 +217,8 @@ class SqlppQueryRewriter implements IQueryRewriter {
             return;
         }
         // Inline column aliases.
-        InlineColumnAliasVisitor inlineColumnAliasVisitor = new InlineColumnAliasVisitor();
-        inlineColumnAliasVisitor.visit(topExpr, false);
+        InlineColumnAliasVisitor inlineColumnAliasVisitor = new InlineColumnAliasVisitor(context);
+        inlineColumnAliasVisitor.visit(topExpr, null);
     }
 
     protected void variableCheckAndRewrite(boolean overwrite) throws AsterixException {

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineColumnAliasVisitor.java
----------------------------------------------------------------------
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 ea0aa86..36786cd 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
@@ -18,204 +18,106 @@
  */
 package org.apache.asterix.lang.sqlpp.rewrites.visitor;
 
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 
 import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.lang.common.base.Expression;
-import org.apache.asterix.lang.common.base.Expression.Kind;
+import org.apache.asterix.lang.common.base.ILangExpression;
 import org.apache.asterix.lang.common.base.Literal;
-import org.apache.asterix.lang.common.clause.GroupbyClause;
+import org.apache.asterix.lang.common.base.Expression.Kind;
 import org.apache.asterix.lang.common.clause.LetClause;
-import org.apache.asterix.lang.common.clause.LimitClause;
-import org.apache.asterix.lang.common.clause.OrderbyClause;
-import org.apache.asterix.lang.common.clause.WhereClause;
-import org.apache.asterix.lang.common.expression.CallExpr;
-import org.apache.asterix.lang.common.expression.FieldAccessor;
 import org.apache.asterix.lang.common.expression.FieldBinding;
-import org.apache.asterix.lang.common.expression.GbyVariableExpressionPair;
-import org.apache.asterix.lang.common.expression.IfExpr;
-import org.apache.asterix.lang.common.expression.IndexAccessor;
-import org.apache.asterix.lang.common.expression.ListConstructor;
 import org.apache.asterix.lang.common.expression.LiteralExpr;
-import org.apache.asterix.lang.common.expression.OperatorExpr;
-import org.apache.asterix.lang.common.expression.QuantifiedExpression;
 import org.apache.asterix.lang.common.expression.RecordConstructor;
-import org.apache.asterix.lang.common.expression.UnaryExpr;
 import org.apache.asterix.lang.common.expression.VariableExpr;
-import org.apache.asterix.lang.common.parser.ScopeChecker;
-import org.apache.asterix.lang.common.rewrites.VariableSubstitutionEnvironment;
-import org.apache.asterix.lang.common.statement.FunctionDecl;
-import org.apache.asterix.lang.common.statement.Query;
-import org.apache.asterix.lang.common.struct.QuantifiedPair;
-import org.apache.asterix.lang.sqlpp.clause.AbstractBinaryCorrelateClause;
-import org.apache.asterix.lang.sqlpp.clause.FromClause;
-import org.apache.asterix.lang.sqlpp.clause.FromTerm;
-import org.apache.asterix.lang.sqlpp.clause.HavingClause;
-import org.apache.asterix.lang.sqlpp.clause.JoinClause;
-import org.apache.asterix.lang.sqlpp.clause.NestClause;
+import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
+import org.apache.asterix.lang.common.struct.VarIdentifier;
 import org.apache.asterix.lang.sqlpp.clause.Projection;
 import org.apache.asterix.lang.sqlpp.clause.SelectBlock;
 import org.apache.asterix.lang.sqlpp.clause.SelectClause;
 import org.apache.asterix.lang.sqlpp.clause.SelectElement;
 import org.apache.asterix.lang.sqlpp.clause.SelectRegular;
-import org.apache.asterix.lang.sqlpp.clause.SelectSetOperation;
-import org.apache.asterix.lang.sqlpp.clause.UnnestClause;
-import org.apache.asterix.lang.sqlpp.expression.CaseExpression;
-import org.apache.asterix.lang.sqlpp.expression.IndependentSubquery;
 import org.apache.asterix.lang.sqlpp.expression.SelectExpression;
-import org.apache.asterix.lang.sqlpp.struct.SetOperationRight;
-import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
-import org.apache.asterix.lang.sqlpp.util.SqlppVariableSubstitutionUtil;
 import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil;
-import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppQueryExpressionVisitor;
-
-public class InlineColumnAliasVisitor extends AbstractSqlppQueryExpressionVisitor<Void, Boolean> {
-
-    private final ScopeChecker scopeChecker = new ScopeChecker();
-
-    @Override
-    public Void visit(WhereClause whereClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        whereClause.getWhereExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(FromClause fromClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (FromTerm fromTerm : fromClause.getFromTerms()) {
-            fromTerm.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
+import org.apache.asterix.lang.sqlpp.visitor.SqlppSubstituteExpressionVisitor;
+import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScopingVisitor;
 
-    @Override
-    public Void visit(FromTerm fromTerm, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        fromTerm.getLeftExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        // A from binding variable will override the alias to substitute.
-        scopeChecker.getCurrentScope().removeSymbolExpressionMapping(fromTerm.getLeftVariable());
-        if (fromTerm.hasPositionalVariable()) {
-            scopeChecker.getCurrentScope().removeSymbolExpressionMapping(fromTerm.getPositionalVariable());
-        }
+public class InlineColumnAliasVisitor extends AbstractSqlppExpressionScopingVisitor {
 
-        for (AbstractBinaryCorrelateClause correlate : fromTerm.getCorrelateClauses()) {
-            correlate.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
+    public InlineColumnAliasVisitor(LangRewritingContext context) {
+        super(context);
     }
 
     @Override
-    public Void visit(JoinClause joinClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        joinClause.getRightExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        removeSubsutitions(joinClause);
-        joinClause.getConditionExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(NestClause nestClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        nestClause.getRightExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        nestClause.getConditionExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        removeSubsutitions(nestClause);
-        return null;
-    }
-
-    @Override
-    public Void visit(UnnestClause unnestClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        unnestClause.getRightExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        removeSubsutitions(unnestClause);
-        return null;
-    }
-
-    @Override
-    public Void visit(Projection projection, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        if (projection.star() || projection.getName() == null) {
-            return null;
-        }
-        projection.getExpression().accept(this, overwriteWithGbyKeyVarRefs);
-        VariableExpr columnAlias =
-                new VariableExpr(SqlppVariableUtil.toInternalVariableIdentifier(projection.getName()));
-        VariableSubstitutionEnvironment env = scopeChecker.getCurrentScope().getVarSubstitutionEnvironment();
-        Expression gbyKey = (Expression) SqlppRewriteUtil.deepCopy(env.findSubstitution(columnAlias));
-        if (overwriteWithGbyKeyVarRefs) {
-            if (gbyKey != null) {
-                projection.setExpression(gbyKey);
-            }
-        } else {
-            scopeChecker.getCurrentScope().addSymbolExpressionMappingToScope(columnAlias, projection.getExpression());
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(SelectBlock selectBlock, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        // Traverses the select block in the order of "select", "group-by",
-        // "group-by" lets and "having".
-        // The first pass over the select clause will not overwrite projection expressions.
-        selectBlock.getSelectClause().accept(this, false);
+    public Expression visit(SelectBlock selectBlock, ILangExpression arg) throws AsterixException {
+        // Gets the map from select clause.
+        Map<Expression, Expression> map = getMap(selectBlock.getSelectClause());
 
+        // Removes all FROM/LET binding variables
         if (selectBlock.hasFromClause()) {
-            selectBlock.getFromClause().accept(this, overwriteWithGbyKeyVarRefs);
+            map.keySet().removeAll(SqlppVariableUtil.getBindingVariables(selectBlock.getFromClause()));
         }
         if (selectBlock.hasLetClauses()) {
-            for (LetClause letClause : selectBlock.getLetList()) {
-                letClause.accept(this, overwriteWithGbyKeyVarRefs);
-            }
+            map.keySet().removeAll(SqlppVariableUtil.getBindingVariables(selectBlock.getLetList()));
         }
+
+        // Creates a substitution visitor.
+        SqlppSubstituteExpressionVisitor visitor = new SqlppSubstituteExpressionVisitor(context, map);
+
+        // Rewrites GROUP BY/LET/HAVING clauses.
         if (selectBlock.hasGroupbyClause()) {
-            selectBlock.getGroupbyClause().accept(this, overwriteWithGbyKeyVarRefs);
+            selectBlock.getGroupbyClause().accept(visitor, arg);
         }
         if (selectBlock.hasLetClausesAfterGroupby()) {
-            for (LetClause letClauseAfterGby : selectBlock.getLetListAfterGroupby()) {
-                letClauseAfterGby.accept(this, true);
+            for (LetClause letClause : selectBlock.getLetListAfterGroupby()) {
+                letClause.accept(visitor, arg);
             }
         }
         if (selectBlock.hasHavingClause()) {
-            selectBlock.getHavingClause().accept(this, overwriteWithGbyKeyVarRefs);
+            selectBlock.getHavingClause().accept(visitor, arg);
         }
+        SelectExpression selectExpression = (SelectExpression) arg;
 
-        // Visit select clause again to overwrite projection expressions to group-by
-        // key variable references if any group-by key is the original projection
-        // column alias.
-        selectBlock.getSelectClause().accept(this, true);
-        return null;
+        // For SET operation queries, column aliases will not substitute ORDER BY nor LIMIT expressions.
+        if (!selectExpression.getSelectSetOperation().hasRightInputs()) {
+            if (selectExpression.hasOrderby()) {
+                selectExpression.getOrderbyClause().accept(visitor, arg);
+            }
+            if (selectExpression.hasLimit()) {
+                selectExpression.getLimitClause().accept(visitor, arg);
+            }
+        }
+        return super.visit(selectBlock, arg);
     }
 
-    @Override
-    public Void visit(SelectClause selectClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
+    private Map<Expression, Expression> getMap(SelectClause selectClause) throws AsterixException {
         if (selectClause.selectElement()) {
-            selectClause.getSelectElement().accept(this, overwriteWithGbyKeyVarRefs);
+            return getMap(selectClause.getSelectElement());
         }
         if (selectClause.selectRegular()) {
-            selectClause.getSelectRegular().accept(this, overwriteWithGbyKeyVarRefs);
+            return getMap(selectClause.getSelectRegular());
         }
         return null;
     }
 
-    @Override
-    public Void visit(SelectElement selectElement, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
+    private Map<Expression, Expression> getMap(SelectElement selectElement) {
         Expression expr = selectElement.getExpression();
-        expr.accept(this, overwriteWithGbyKeyVarRefs);
         if (expr.getKind() == Kind.RECORD_CONSTRUCTOR_EXPRESSION) {
             // Rewrite top-level field names (aliases), in order to be consistent with SelectRegular.
-            mapForRecordConstructor(overwriteWithGbyKeyVarRefs, (RecordConstructor) expr);
+            return mapRecordConstructor((RecordConstructor) expr);
         }
-        return null;
+        return Collections.emptyMap();
     }
 
-    /**
-     * Map aliases for a record constructor in SELECT ELEMENT.
-     *
-     * @param overwriteWithGbyKeyVarRefs,
-     *            whether we rewrite the record constructor with mapped group-by key variables.
-     * @param rc,
-     *            the RecordConstructor expression.
-     * @throws AsterixException
-     */
-    private void mapForRecordConstructor(Boolean overwriteWithGbyKeyVarRefs, RecordConstructor rc)
-            throws AsterixException {
+    private Map<Expression, Expression> getMap(SelectRegular selectRegular) {
+        return mapProjections(selectRegular.getProjections());
+    }
+
+    private Map<Expression, Expression> mapRecordConstructor(RecordConstructor rc) {
+        Map<Expression, Expression> exprMap = new HashMap<>();
         for (FieldBinding binding : rc.getFbList()) {
             Expression leftExpr = binding.getLeftExpr();
             // We only need to deal with the case that the left expression (for a field name) is
@@ -226,263 +128,20 @@ public class InlineColumnAliasVisitor extends AbstractSqlppQueryExpressionVisito
             }
             LiteralExpr literalExpr = (LiteralExpr) leftExpr;
             if (literalExpr.getValue().getLiteralType() == Literal.Type.STRING) {
-                String fieldName = literalExpr.getValue().getStringValue();
-                VariableExpr columnAlias = new VariableExpr(SqlppVariableUtil.toInternalVariableIdentifier(fieldName));
-                VariableSubstitutionEnvironment env = scopeChecker.getCurrentScope().getVarSubstitutionEnvironment();
-                if (overwriteWithGbyKeyVarRefs) {
-                    // Rewrites the field value expression by the mapped grouping key
-                    // (for the column alias) if there exists such a mapping.
-                    Expression gbyKey = (Expression) SqlppRewriteUtil.deepCopy(env.findSubstitution(columnAlias));
-                    if (gbyKey != null) {
-                        binding.setRightExpr(gbyKey);
-                    }
-                } else {
-                    // If this is the first pass, map a field name (i.e., column alias) to the field expression.
-                    scopeChecker.getCurrentScope().addSymbolExpressionMappingToScope(columnAlias,
-                            binding.getRightExpr());
-                }
-            }
-        }
-    }
-
-    @Override
-    public Void visit(SelectRegular selectRegular, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (Projection projection : selectRegular.getProjections()) {
-            projection.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(SelectSetOperation selectSetOperation, Boolean overwriteWithGbyKeyVarRefs)
-            throws AsterixException {
-        selectSetOperation.getLeftInput().accept(this, overwriteWithGbyKeyVarRefs);
-        for (SetOperationRight right : selectSetOperation.getRightInputs()) {
-            right.getSetOperationRightInput().accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(SelectExpression selectExpression, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        scopeChecker.createNewScope();
-
-        // Visits let bindings.
-        if (selectExpression.hasLetClauses()) {
-            for (LetClause lc : selectExpression.getLetList()) {
-                lc.accept(this, overwriteWithGbyKeyVarRefs);
+                String fieldName = SqlppVariableUtil.toInternalVariableName(literalExpr.getValue().getStringValue());
+                exprMap.put(new VariableExpr(new VarIdentifier(fieldName)), binding.getRightExpr());
             }
         }
-
-        // Visits selectSetOperation
-        SelectSetOperation selectSetOperation = selectExpression.getSelectSetOperation();
-        selectSetOperation.accept(this, overwriteWithGbyKeyVarRefs);
-
-        // If there is a UNION in the selectSetOperation, we cannot overwrite order by or limit.
-        if (!selectSetOperation.hasRightInputs()) {
-            // Visits order by.
-            if (selectExpression.hasOrderby()) {
-                selectExpression.getOrderbyClause().accept(this, overwriteWithGbyKeyVarRefs);
-            }
-            // Visits limit.
-            if (selectExpression.hasLimit()) {
-                selectExpression.getLimitClause().accept(this, overwriteWithGbyKeyVarRefs);
-            }
-        }
-
-        // Exits the scope that were entered within this select expression
-        scopeChecker.removeCurrentScope();
-        return null;
-    }
-
-    @Override
-    public Void visit(LetClause letClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        VariableSubstitutionEnvironment env = scopeChecker.getCurrentScope().getVarSubstitutionEnvironment();
-        if (overwriteWithGbyKeyVarRefs) {
-            Expression newBindExpr = (Expression) SqlppVariableSubstitutionUtil
-                    .substituteVariableWithoutContext(letClause.getBindingExpr(), env);
-            letClause.setBindingExpr(newBindExpr);
-        }
-        letClause.getBindingExpr().accept(this, false);
-        // A let binding variable will override the alias to substitute.
-        scopeChecker.getCurrentScope().removeSymbolExpressionMapping(letClause.getVarExpr());
-        return null;
-    }
-
-    @Override
-    public Void visit(OrderbyClause oc, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        VariableSubstitutionEnvironment env = scopeChecker.getCurrentScope().getVarSubstitutionEnvironment();
-        List<Expression> orderExprs = new ArrayList<>();
-        for (Expression orderExpr : oc.getOrderbyList()) {
-            orderExprs.add((Expression) SqlppVariableSubstitutionUtil.substituteVariableWithoutContext(orderExpr, env));
-            orderExpr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        oc.setOrderbyList(orderExprs);
-        return null;
-    }
-
-    @Override
-    public Void visit(GroupbyClause gc, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        VariableSubstitutionEnvironment env = scopeChecker.getCurrentScope().getVarSubstitutionEnvironment();
-        Map<VariableExpr, VariableExpr> oldGbyExprsToNewGbyVarMap = new HashMap<>();
-        for (GbyVariableExpressionPair gbyVarExpr : gc.getGbyPairList()) {
-            Expression oldGbyExpr = gbyVarExpr.getExpr();
-            Expression newExpr =
-                    (Expression) SqlppVariableSubstitutionUtil.substituteVariableWithoutContext(oldGbyExpr, env);
-            newExpr.accept(this, overwriteWithGbyKeyVarRefs);
-            gbyVarExpr.setExpr(newExpr);
-            if (oldGbyExpr.getKind() == Kind.VARIABLE_EXPRESSION) {
-                VariableExpr oldGbyVarExpr = (VariableExpr) oldGbyExpr;
-                if (env.findSubstitution(oldGbyVarExpr) != null) {
-                    // Re-mapping that needs to be added.
-                    oldGbyExprsToNewGbyVarMap.put(oldGbyVarExpr, gbyVarExpr.getVar());
-                }
-            }
-        }
-        for (Entry<VariableExpr, VariableExpr> entry : oldGbyExprsToNewGbyVarMap.entrySet()) {
-            // The group-by key variable will override the alias to substitute.
-            scopeChecker.getCurrentScope().removeSymbolExpressionMapping(entry.getKey());
-            scopeChecker.getCurrentScope().addSymbolExpressionMappingToScope(entry.getKey(), entry.getValue());
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(LimitClause limitClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        limitClause.getLimitExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(HavingClause havingClause, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        VariableSubstitutionEnvironment env = scopeChecker.getCurrentScope().getVarSubstitutionEnvironment();
-        Expression newFilterExpr = (Expression) SqlppVariableSubstitutionUtil
-                .substituteVariableWithoutContext(havingClause.getFilterExpression(), env);
-        newFilterExpr.accept(this, overwriteWithGbyKeyVarRefs);
-        havingClause.setFilterExpression(newFilterExpr);
-        return null;
-    }
-
-    @Override
-    public Void visit(Query q, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        q.getBody().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(FunctionDecl fd, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        scopeChecker.createNewScope();
-        fd.getFuncBody().accept(this, overwriteWithGbyKeyVarRefs);
-        scopeChecker.removeCurrentScope();
-        return null;
-    }
-
-    @Override
-    public Void visit(LiteralExpr l, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        return null;
-    }
-
-    @Override
-    public Void visit(ListConstructor lc, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (Expression expr : lc.getExprList()) {
-            expr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(RecordConstructor rc, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (FieldBinding binding : rc.getFbList()) {
-            binding.getLeftExpr().accept(this, false);
-            binding.getRightExpr().accept(this, false);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(OperatorExpr operatorExpr, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (Expression expr : operatorExpr.getExprList()) {
-            expr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(IfExpr ifExpr, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        ifExpr.getCondExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        ifExpr.getThenExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        ifExpr.getElseExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(QuantifiedExpression qe, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (QuantifiedPair pair : qe.getQuantifiedList()) {
-            pair.getExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        qe.getSatisfiesExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(CallExpr callExpr, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        for (Expression expr : callExpr.getExprList()) {
-            expr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(VariableExpr varExpr, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        return null;
-    }
-
-    @Override
-    public Void visit(UnaryExpr u, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        u.getExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(FieldAccessor fa, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        fa.getExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(IndexAccessor ia, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        ia.getExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        Expression indexExpr = ia.getExpr();
-        if (indexExpr != null) {
-            indexExpr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        return null;
-    }
-
-    @Override
-    public Void visit(IndependentSubquery independentSubquery, Boolean overwriteWithGbyKeyVarRefs)
-            throws AsterixException {
-        independentSubquery.getExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
-    }
-
-    @Override
-    public Void visit(CaseExpression caseExpression, Boolean overwriteWithGbyKeyVarRefs) throws AsterixException {
-        caseExpression.getConditionExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        for (Expression expr : caseExpression.getWhenExprs()) {
-            expr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        for (Expression expr : caseExpression.getThenExprs()) {
-            expr.accept(this, overwriteWithGbyKeyVarRefs);
-        }
-        caseExpression.getElseExpr().accept(this, overwriteWithGbyKeyVarRefs);
-        return null;
+        return exprMap;
     }
 
-    private void removeSubsutitions(AbstractBinaryCorrelateClause unnestClause) {
-        scopeChecker.getCurrentScope().removeSymbolExpressionMapping(unnestClause.getRightVariable());
-        if (unnestClause.hasPositionalVariable()) {
-            scopeChecker.getCurrentScope().removeSymbolExpressionMapping(unnestClause.getPositionalVariable());
+    private Map<Expression, Expression> mapProjections(List<Projection> projections) {
+        Map<Expression, Expression> exprMap = new HashMap<>();
+        for (Projection projection : projections) {
+            exprMap.put(
+                    new VariableExpr(new VarIdentifier(SqlppVariableUtil.toInternalVariableName(projection.getName()))),
+                    projection.getExpression());
         }
+        return exprMap;
     }
 }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineWithExpressionVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineWithExpressionVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineWithExpressionVisitor.java
index a01c09f..7a4f0fe 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineWithExpressionVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/InlineWithExpressionVisitor.java
@@ -18,6 +18,8 @@
  */
 package org.apache.asterix.lang.sqlpp.rewrites.visitor;
 
+import static org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil.substituteExpression;
+
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -27,37 +29,45 @@ import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.lang.common.base.Expression;
 import org.apache.asterix.lang.common.base.ILangExpression;
 import org.apache.asterix.lang.common.clause.LetClause;
-import org.apache.asterix.lang.common.expression.VariableExpr;
-import org.apache.asterix.lang.sqlpp.expression.IndependentSubquery;
+import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
 import org.apache.asterix.lang.sqlpp.expression.SelectExpression;
-import org.apache.asterix.lang.sqlpp.util.SqlppVariableSubstitutionUtil;
-import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppSimpleExpressionVisitor;
+import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
+import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScopingVisitor;
+
+public class InlineWithExpressionVisitor extends AbstractSqlppExpressionScopingVisitor {
 
-public class InlineWithExpressionVisitor extends AbstractSqlppSimpleExpressionVisitor {
+    public InlineWithExpressionVisitor(LangRewritingContext context) {
+        super(context);
+    }
 
     @Override
     public Expression visit(SelectExpression selectExpression, ILangExpression arg) throws AsterixException {
         if (selectExpression.hasLetClauses()) {
             // Inlines the leading WITH list.
-            Map<VariableExpr, Expression> varExprMap = new HashMap<>();
+            Map<Expression, Expression> varExprMap = new HashMap<>();
             List<LetClause> withs = selectExpression.getLetList();
             Iterator<LetClause> with = withs.iterator();
             while (with.hasNext()) {
                 LetClause letClause = with.next();
                 // Replaces the let binding Expr.
                 Expression expr = letClause.getBindingExpr();
-                letClause.setBindingExpr(
-                        (Expression) SqlppVariableSubstitutionUtil.substituteVariableWithoutContext(expr, varExprMap));
+                Expression newBindingExpr = SqlppRewriteUtil.substituteExpression(expr, varExprMap, context);
+                letClause.setBindingExpr(newBindingExpr);
+
+                // Performs the rewriting recursively in the newBindingExpr itself.
+                super.visit(newBindingExpr, arg);
+
+                // Removes the WITH entry and adds variable-expr mapping into the varExprMap.
                 with.remove();
                 Expression bindingExpr = letClause.getBindingExpr();
                 // Wraps the binding expression with IndependentSubquery, so that free identifier references
                 // in the binding expression will not be resolved use outer-scope variables.
-                varExprMap.put(letClause.getVarExpr(), new IndependentSubquery(bindingExpr));
+                varExprMap.put(letClause.getVarExpr(), bindingExpr);
             }
 
             // Inlines WITH expressions into the select expression.
-            SelectExpression newSelectExpression = (SelectExpression) SqlppVariableSubstitutionUtil
-                    .substituteVariableWithoutContext(selectExpression, varExprMap);
+            SelectExpression newSelectExpression = (SelectExpression) substituteExpression(selectExpression,
+                    varExprMap, context);
 
             // Continues to visit the rewritten select expression.
             return super.visit(newSelectExpression, arg);

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGlobalAggregationSugarVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGlobalAggregationSugarVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGlobalAggregationSugarVisitor.java
index 98d18c2..6308d7f 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGlobalAggregationSugarVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGlobalAggregationSugarVisitor.java
@@ -19,6 +19,7 @@
 package org.apache.asterix.lang.sqlpp.rewrites.visitor;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 
 import org.apache.asterix.common.exceptions.AsterixException;
@@ -27,7 +28,6 @@ import org.apache.asterix.lang.common.base.ILangExpression;
 import org.apache.asterix.lang.common.clause.GroupbyClause;
 import org.apache.asterix.lang.common.expression.GbyVariableExpressionPair;
 import org.apache.asterix.lang.common.expression.LiteralExpr;
-import org.apache.asterix.lang.common.expression.VariableExpr;
 import org.apache.asterix.lang.common.literal.IntegerLiteral;
 import org.apache.asterix.lang.sqlpp.clause.SelectBlock;
 import org.apache.asterix.lang.sqlpp.clause.SelectClause;
@@ -51,9 +51,8 @@ public class SqlppGlobalAggregationSugarVisitor extends AbstractSqlppSimpleExpre
                 List<GbyVariableExpressionPair> gbyPairList = new ArrayList<>();
                 gbyPairList.add(new GbyVariableExpressionPair(null, new LiteralExpr(new IntegerLiteral(1))));
                 List<GbyVariableExpressionPair> decorPairList = new ArrayList<>();
-                List<VariableExpr> withVarList = new ArrayList<>();
-                GroupbyClause gbyClause = new GroupbyClause(gbyPairList, decorPairList, withVarList, null, null, false,
-                        true);
+                GroupbyClause gbyClause = new GroupbyClause(gbyPairList, decorPairList, new HashMap<>(), null, null,
+                        false, true);
                 selectBlock.setGroupbyClause(gbyClause);
             }
         }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupBySugarVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupBySugarVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupBySugarVisitor.java
index 47aa02b..996a2ec 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupBySugarVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupBySugarVisitor.java
@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -30,7 +29,6 @@ import java.util.Set;
 import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.common.functions.FunctionSignature;
 import org.apache.asterix.lang.common.base.Expression;
-import org.apache.asterix.lang.common.base.Expression.Kind;
 import org.apache.asterix.lang.common.base.ILangExpression;
 import org.apache.asterix.lang.common.expression.CallExpr;
 import org.apache.asterix.lang.common.expression.FieldAccessor;
@@ -46,7 +44,6 @@ import org.apache.asterix.lang.sqlpp.expression.SelectExpression;
 import org.apache.asterix.lang.sqlpp.struct.SetOperationInput;
 import org.apache.asterix.lang.sqlpp.util.FunctionMapUtil;
 import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
-import org.apache.asterix.lang.sqlpp.util.SqlppVariableSubstitutionUtil;
 import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil;
 import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScopingVisitor;
 
@@ -83,24 +80,20 @@ import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScoping
 public class SqlppGroupBySugarVisitor extends AbstractSqlppExpressionScopingVisitor {
 
     private final Expression groupVar;
-    private final Collection<VariableExpr> targetVars;
-    private final Collection<VariableExpr> allVisableVars;
+    private final Collection<VariableExpr> fieldVars;
 
     public SqlppGroupBySugarVisitor(LangRewritingContext context, Expression groupVar,
-            Collection<VariableExpr> targetVars, Collection<VariableExpr> allVisableVars) {
+            Collection<VariableExpr> fieldVars) {
         super(context);
         this.groupVar = groupVar;
-        this.targetVars = targetVars;
-        this.allVisableVars = allVisableVars;
-        allVisableVars.remove(groupVar);
+        this.fieldVars = fieldVars;
     }
 
     @Override
     public Expression visit(CallExpr callExpr, ILangExpression arg) throws AsterixException {
         List<Expression> newExprList = new ArrayList<>();
         FunctionSignature signature = callExpr.getFunctionSignature();
-        boolean aggregate = FunctionMapUtil.isSql92AggregateFunction(signature)
-                || FunctionMapUtil.isCoreAggregateFunction(signature);
+        boolean aggregate = FunctionMapUtil.isSql92AggregateFunction(signature);
         boolean rewritten = false;
         for (Expression expr : callExpr.getExprList()) {
             Expression newExpr = aggregate ? wrapAggregationArgument(expr) : expr;
@@ -118,54 +111,32 @@ public class SqlppGroupBySugarVisitor extends AbstractSqlppExpressionScopingVisi
 
     private Expression wrapAggregationArgument(Expression argExpr) throws AsterixException {
         Expression expr = argExpr;
-        if (expr.getKind() == Kind.SELECT_EXPRESSION) {
-            return expr;
-        }
-        Set<VariableExpr> definedVars = scopeChecker.getCurrentScope().getLiveVariables();
-        allVisableVars.addAll(definedVars);
         Set<VariableExpr> freeVars = SqlppRewriteUtil.getFreeVariable(expr);
 
-        // Whether we need to resolve undefined variables.
-        boolean needResolve = !allVisableVars.containsAll(freeVars);
+        VariableExpr fromBindingVar = new VariableExpr(context.newVariable());
+        FromTerm fromTerm = new FromTerm(groupVar, fromBindingVar, null, null);
+        FromClause fromClause = new FromClause(Collections.singletonList(fromTerm));
 
-        Set<VariableExpr> vars = new HashSet<>(targetVars);
-        vars.removeAll(definedVars); // Exclude re-defined local variables.
-        if (!needResolve && !vars.containsAll(freeVars)) {
-            return expr;
+        // Maps field variable expressions to field accesses.
+        Map<Expression, Expression> varExprMap = new HashMap<>();
+        for (VariableExpr usedVar : freeVars) {
+            // Reference to a field in the group variable.
+            if (fieldVars.contains(usedVar)) {
+                // Rewrites to a reference to a field in the group variable.
+                varExprMap.put(usedVar,
+                                new FieldAccessor(fromBindingVar, SqlppVariableUtil.toUserDefinedVariableName(usedVar
+                                        .getVar())));
+            }
         }
 
-        VariableExpr var = new VariableExpr(context.newVariable());
-        FromTerm fromTerm = new FromTerm(groupVar, var, null, null);
-        FromClause fromClause = new FromClause(Collections.singletonList(fromTerm));
-
         // Select clause.
-        SelectElement selectElement = new SelectElement(expr);
+        SelectElement selectElement = new SelectElement(
+                SqlppRewriteUtil.substituteExpression(expr, varExprMap, context));
         SelectClause selectClause = new SelectClause(selectElement, null, false);
 
         // Construct the select expression.
         SelectBlock selectBlock = new SelectBlock(selectClause, fromClause, null, null, null, null, null);
         SelectSetOperation selectSetOperation = new SelectSetOperation(new SetOperationInput(selectBlock, null), null);
-        SelectExpression selectExpression = new SelectExpression(null, selectSetOperation, null, null, true);
-
-        // replace variable expressions with field access
-        Map<VariableExpr, Expression> varExprMap = new HashMap<>();
-        for (VariableExpr usedVar : freeVars) {
-            if (allVisableVars.contains(usedVar)) {
-                // Reference to a defined variable.
-                if (vars.contains(usedVar)) {
-                    // Reference to a variable defined before the group-by,
-                    // i.e., not a variable defined by a LET after the group-by.
-                    varExprMap.put(usedVar,
-                            new FieldAccessor(var, SqlppVariableUtil.toUserDefinedVariableName(usedVar.getVar())));
-                }
-            } else {
-                // Reference to an undefined variable.
-                varExprMap.put(usedVar,
-                        this.wrapWithResolveFunction(usedVar, new HashSet<>(Collections.singleton(var))));
-            }
-        }
-        selectElement.setExpression(
-                (Expression) SqlppVariableSubstitutionUtil.substituteVariableWithoutContext(expr, varExprMap));
-        return selectExpression;
+        return new SelectExpression(null, selectSetOperation, null, null, true);
     }
 }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByVisitor.java
index 6575752..0170709 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppGroupByVisitor.java
@@ -19,9 +19,11 @@
 package org.apache.asterix.lang.sqlpp.rewrites.visitor;
 
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.asterix.common.exceptions.AsterixException;
@@ -29,13 +31,17 @@ import org.apache.asterix.lang.common.base.Expression;
 import org.apache.asterix.lang.common.base.ILangExpression;
 import org.apache.asterix.lang.common.clause.GroupbyClause;
 import org.apache.asterix.lang.common.clause.LetClause;
-import org.apache.asterix.lang.common.context.Scope;
+import org.apache.asterix.lang.common.clause.LimitClause;
+import org.apache.asterix.lang.common.clause.OrderbyClause;
 import org.apache.asterix.lang.common.expression.GbyVariableExpressionPair;
 import org.apache.asterix.lang.common.expression.VariableExpr;
 import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
 import org.apache.asterix.lang.common.struct.Identifier;
 import org.apache.asterix.lang.common.struct.VarIdentifier;
+import org.apache.asterix.lang.sqlpp.clause.FromClause;
+import org.apache.asterix.lang.sqlpp.clause.HavingClause;
 import org.apache.asterix.lang.sqlpp.clause.SelectBlock;
+import org.apache.asterix.lang.sqlpp.clause.SelectClause;
 import org.apache.asterix.lang.sqlpp.expression.SelectExpression;
 import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
 import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil;
@@ -88,8 +94,9 @@ public class SqlppGroupByVisitor extends AbstractSqlppExpressionScopingVisitor {
     public Expression visit(SelectBlock selectBlock, ILangExpression arg) throws AsterixException {
         // Traverses the select block in the order of "from", "let"s, "where",
         // "group by", "let"s, "having" and "select".
+        FromClause fromClause = selectBlock.getFromClause();
         if (selectBlock.hasFromClause()) {
-            selectBlock.getFromClause().accept(this, arg);
+            fromClause.accept(this, arg);
         }
         if (selectBlock.hasLetClauses()) {
             List<LetClause> letList = selectBlock.getLetList();
@@ -101,48 +108,110 @@ public class SqlppGroupByVisitor extends AbstractSqlppExpressionScopingVisitor {
             selectBlock.getWhereClause().accept(this, arg);
         }
         if (selectBlock.hasGroupbyClause()) {
-            selectBlock.getGroupbyClause().accept(this, arg);
-            Set<VariableExpr> withVarSet = new HashSet<>(selectBlock.getGroupbyClause().getWithVarList());
-            withVarSet.remove(selectBlock.getGroupbyClause().getGroupVar());
+            GroupbyClause groupbyClause = selectBlock.getGroupbyClause();
+            groupbyClause.accept(this, fromClause);
+            Collection<VariableExpr> visibleVarsInCurrentScope = SqlppVariableUtil.getBindingVariables(groupbyClause);
 
-            Set<VariableExpr> allVisableVars = SqlppVariableUtil
-                    .getLiveVariables(scopeChecker.getCurrentScope());
+            VariableExpr groupVar = groupbyClause.getGroupVar();
+            Set<VariableExpr> groupFieldVars = getGroupFieldVariables(groupbyClause);
+
+            Collection<VariableExpr> freeVariablesInGbyLets = new HashSet<>();
             if (selectBlock.hasLetClausesAfterGroupby()) {
                 List<LetClause> letListAfterGby = selectBlock.getLetListAfterGroupby();
                 for (LetClause letClauseAfterGby : letListAfterGby) {
                     letClauseAfterGby.accept(this, arg);
                     // Rewrites each let clause after the group-by.
-                    SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(selectBlock.getGroupbyClause().getGroupVar(),
-                            withVarSet, allVisableVars, letClauseAfterGby, context);
-                    // Adds let vars to all visiable vars.
-                    allVisableVars.add(letClauseAfterGby.getVarExpr());
+                    SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(groupVar, groupFieldVars, letClauseAfterGby,
+                            context);
+                    Collection<VariableExpr> freeVariablesInLet = SqlppVariableUtil.getFreeVariables(letClauseAfterGby
+                            .getBindingExpr());
+                    freeVariablesInLet.removeAll(visibleVarsInCurrentScope);
+                    freeVariablesInGbyLets.addAll(freeVariablesInLet);
+                    visibleVarsInCurrentScope.add(letClauseAfterGby.getVarExpr());
                 }
             }
 
+            Collection<VariableExpr> freeVariables = new HashSet<>();
             if (selectBlock.hasHavingClause()) {
-                selectBlock.getHavingClause().accept(this, arg);
                 // Rewrites the having clause.
-                SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(selectBlock.getGroupbyClause().getGroupVar(),
-                        withVarSet, allVisableVars, selectBlock.getHavingClause(), context);
+                HavingClause havingClause = selectBlock.getHavingClause();
+                havingClause.accept(this, arg);
+                SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(groupVar, groupFieldVars, havingClause, context);
+                freeVariables.addAll(SqlppVariableUtil.getFreeVariables(havingClause));
             }
 
             SelectExpression parentSelectExpression = (SelectExpression) arg;
-            if (parentSelectExpression.hasOrderby()) {
-                // Rewrites the order-by clause.
-                SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(selectBlock.getGroupbyClause().getGroupVar(),
-                        withVarSet, allVisableVars, parentSelectExpression.getOrderbyClause(), context);
-            }
-            if (parentSelectExpression.hasLimit()) {
-                // Rewrites the limit clause.
-                SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(selectBlock.getGroupbyClause().getGroupVar(),
-                        withVarSet, allVisableVars, parentSelectExpression.getLimitClause(), context);
+            // We cannot rewrite ORDER BY and LIMIT if it's a SET operation query.
+            if (!parentSelectExpression.getSelectSetOperation().hasRightInputs()) {
+                if (parentSelectExpression.hasOrderby()) {
+                    // Rewrites the ORDER BY clause.
+                    OrderbyClause orderbyClause = parentSelectExpression.getOrderbyClause();
+                    orderbyClause.accept(this, arg);
+                    SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(groupVar, groupFieldVars, orderbyClause,
+                            context);
+                    freeVariables.addAll(SqlppVariableUtil.getFreeVariables(orderbyClause));
+                }
+                if (parentSelectExpression.hasLimit()) {
+                    // Rewrites the LIMIT clause.
+                    LimitClause limitClause = parentSelectExpression.getLimitClause();
+                    limitClause.accept(this, arg);
+                    SqlppRewriteUtil
+                            .rewriteExpressionUsingGroupVariable(groupVar, groupFieldVars, limitClause, context);
+                    freeVariables.addAll(SqlppVariableUtil.getFreeVariables(limitClause));
+                }
             }
 
             // Visits the select clause.
-            selectBlock.getSelectClause().accept(this, arg);
+            SelectClause selectClause = selectBlock.getSelectClause();
+            selectClause.accept(this, arg);
             // Rewrites the select clause.
-            SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(selectBlock.getGroupbyClause().getGroupVar(),
-                    withVarSet, allVisableVars, selectBlock.getSelectClause(), context);
+            SqlppRewriteUtil.rewriteExpressionUsingGroupVariable(groupVar, groupFieldVars, selectClause, context);
+            freeVariables.addAll(SqlppVariableUtil.getFreeVariables(selectClause));
+            freeVariables.removeAll(visibleVarsInCurrentScope);
+
+            // Gets the final free variables.
+            freeVariables.addAll(freeVariablesInGbyLets);
+
+            // Gets outer scope variables.
+            Collection<VariableExpr> decorVars = SqlppVariableUtil.getLiveVariables(
+                    scopeChecker.getCurrentScope(), true);
+            decorVars.removeAll(visibleVarsInCurrentScope);
+
+            // Need path resolution or not?
+            boolean needResolution = !decorVars.containsAll(freeVariables);
+            // If path resolution is needed, we need to include all outer scope variables in the decoration list.
+            // Otherwise, we only need to retain used free variables.
+            if (needResolution) {
+                // Tracks used variables, including WITH variables.
+                decorVars.retainAll(freeVariables);
+                // Adds all non-WITH outer scope variables, for path resolution.
+                Collection<VariableExpr> visibleOuterScopeNonWithVars = SqlppVariableUtil.getLiveVariables(
+                        scopeChecker.getCurrentScope(), false);
+                visibleOuterScopeNonWithVars.removeAll(visibleVarsInCurrentScope);
+                decorVars.addAll(visibleOuterScopeNonWithVars);
+            } else {
+                // Only retains used free variables.
+                decorVars.retainAll(freeVariables);
+            }
+            if (!decorVars.isEmpty()) {
+                // Adds used WITH variables.
+                Collection<VariableExpr> visibleOuterScopeNonWithVars = SqlppVariableUtil.getLiveVariables(
+                        scopeChecker.getCurrentScope(), false);
+                visibleOuterScopeNonWithVars.retainAll(freeVariables);
+                decorVars.addAll(visibleOuterScopeNonWithVars);
+
+                // Adds necessary decoration variables for the GROUP BY.
+                // NOTE: we need to include WITH binding variables so as they can be evaluated before
+                // the GROUP BY instead of being inlined as part of nested pipepline. The current optimzier
+                // is not able to optimize the latter case. The following query is such an example:
+                // asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dapd/q2-11
+                List<GbyVariableExpressionPair> decorList = new ArrayList<>();
+                for (VariableExpr var : decorVars) {
+                    decorList.add(new GbyVariableExpressionPair((VariableExpr) SqlppRewriteUtil.deepCopy(var),
+                            (Expression) SqlppRewriteUtil.deepCopy(var)));
+                }
+                groupbyClause.getDecorPairList().addAll(decorList);
+            }
         } else {
             selectBlock.getSelectClause().accept(this, arg);
         }
@@ -151,43 +220,35 @@ public class SqlppGroupByVisitor extends AbstractSqlppExpressionScopingVisitor {
 
     @Override
     public Expression visit(GroupbyClause gc, ILangExpression arg) throws AsterixException {
-        Scope newScope = scopeChecker.extendCurrentScopeNoPush(true);
-        // Puts all group-by variables into the symbol set of the new scope.
-        for (GbyVariableExpressionPair gbyVarExpr : gc.getGbyPairList()) {
-            gbyVarExpr.setExpr(gbyVarExpr.getExpr().accept(this, arg));
-            VariableExpr gbyVar = gbyVarExpr.getVar();
-            if (gbyVar != null) {
-                newScope.addNewVarSymbolToScope(gbyVarExpr.getVar().getVar());
-            }
-        }
-        // Puts all live variables into withVarList.
-        List<VariableExpr> withVarList = new ArrayList<>();
-        Iterator<Identifier> varIterator = scopeChecker.getCurrentScope().liveSymbols();
-        while (varIterator.hasNext()) {
-            Identifier ident = varIterator.next();
+        // Puts all FROM binding variables into withVarList.
+        FromClause fromClause = (FromClause) arg;
+        Collection<VariableExpr> fromBindingVars =
+                fromClause == null ? new ArrayList<>() : SqlppVariableUtil.getBindingVariables(fromClause);
+        Map<Expression, VariableExpr> withVarMap = new HashMap<>();
+        for (VariableExpr fromBindingVar : fromBindingVars) {
             VariableExpr varExpr = new VariableExpr();
-            if (ident instanceof VarIdentifier) {
-                varExpr.setIsNewVar(false);
-                varExpr.setVar((VarIdentifier) ident);
-                withVarList.add(varExpr);
-                newScope.addNewVarSymbolToScope((VarIdentifier) ident);
-            }
+            varExpr.setIsNewVar(false);
+            varExpr.setVar(fromBindingVar.getVar());
+            VariableExpr newVarExpr = (VariableExpr) SqlppRewriteUtil.deepCopy(varExpr);
+            withVarMap.put(varExpr, newVarExpr);
         }
-
         // Sets the field list for the group variable.
         List<Pair<Expression, Identifier>> groupFieldList = new ArrayList<>();
         if (!gc.hasGroupFieldList()) {
-            for (VariableExpr varExpr : withVarList) {
+            for (VariableExpr varExpr : fromBindingVars) {
                 Pair<Expression, Identifier> varIdPair = new Pair<>(new VariableExpr(varExpr.getVar()),
                         SqlppVariableUtil.toUserDefinedVariableName(varExpr.getVar()));
                 groupFieldList.add(varIdPair);
             }
             gc.setGroupFieldList(groupFieldList);
         } else {
-            // Check the scopes of group field variables.
             for (Pair<Expression, Identifier> groupField : gc.getGroupFieldList()) {
-                Expression newVar = groupField.first.accept(this, arg);
-                groupFieldList.add(new Pair<>(newVar, groupField.second));
+                Expression newFieldExpr = groupField.first.accept(this, arg);
+                groupFieldList.add(new Pair<>(newFieldExpr, groupField.second));
+                // Adds a field binding variable into withVarList.
+                VariableExpr bindingVar = new VariableExpr(
+                        new VarIdentifier(SqlppVariableUtil.toInternalVariableName(groupField.second.getValue())));
+                withVarMap.put(newFieldExpr, bindingVar);
             }
         }
         gc.setGroupFieldList(groupFieldList);
@@ -197,15 +258,25 @@ public class SqlppGroupByVisitor extends AbstractSqlppExpressionScopingVisitor {
             VariableExpr groupVar = new VariableExpr(context.newVariable());
             gc.setGroupVar(groupVar);
         }
-        newScope.addNewVarSymbolToScope(gc.getGroupVar().getVar());
 
         // Adds the group variable into the "with" (i.e., re-binding) variable list.
         VariableExpr gbyVarRef = new VariableExpr(gc.getGroupVar().getVar());
         gbyVarRef.setIsNewVar(false);
-        withVarList.add(gbyVarRef);
-        gc.setWithVarList(withVarList);
+        withVarMap.put(gbyVarRef, (VariableExpr) SqlppRewriteUtil.deepCopy(gbyVarRef));
+        gc.setWithVarMap(withVarMap);
 
-        scopeChecker.replaceCurrentScope(newScope);
-        return null;
+        // Call super.visit(...) to scope variables.
+        return super.visit(gc, arg);
+    }
+
+    private Set<VariableExpr> getGroupFieldVariables(GroupbyClause groupbyClause) {
+        Set<VariableExpr> fieldVars = new HashSet<>();
+        if (groupbyClause.hasGroupFieldList()) {
+            for (Pair<Expression, Identifier> groupField : groupbyClause.getGroupFieldList()) {
+                fieldVars.add(new VariableExpr(new VarIdentifier(SqlppVariableUtil
+                        .toInternalVariableName(groupField.second.getValue()))));
+            }
+        }
+        return fieldVars;
     }
 }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SqlppInlineUdfsVisitor.java
----------------------------------------------------------------------
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 d2730ff..c765726 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
@@ -26,7 +26,6 @@ import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.lang.common.base.Expression;
 import org.apache.asterix.lang.common.base.IRewriterFactory;
 import org.apache.asterix.lang.common.clause.LetClause;
-import org.apache.asterix.lang.common.expression.VariableExpr;
 import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
 import org.apache.asterix.lang.common.statement.FunctionDecl;
 import org.apache.asterix.lang.common.visitor.AbstractInlineUdfsVisitor;
@@ -47,7 +46,7 @@ import org.apache.asterix.lang.sqlpp.expression.CaseExpression;
 import org.apache.asterix.lang.sqlpp.expression.IndependentSubquery;
 import org.apache.asterix.lang.sqlpp.expression.SelectExpression;
 import org.apache.asterix.lang.sqlpp.struct.SetOperationRight;
-import org.apache.asterix.lang.sqlpp.util.SqlppVariableSubstitutionUtil;
+import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
 import org.apache.asterix.lang.sqlpp.visitor.SqlppCloneAndSubstituteVariablesVisitor;
 import org.apache.asterix.lang.sqlpp.visitor.base.ISqlppVisitor;
 import org.apache.asterix.metadata.declared.AqlMetadataProvider;
@@ -75,8 +74,8 @@ public class SqlppInlineUdfsVisitor extends AbstractInlineUdfsVisitor
     @Override
     protected Expression generateQueryExpression(List<LetClause> letClauses, Expression returnExpr)
             throws AsterixException {
-        Map<VariableExpr, Expression> varExprMap = extractLetBindingVariableExpressionMappings(letClauses);
-        return (Expression) SqlppVariableSubstitutionUtil.substituteVariableWithoutContext(returnExpr, varExprMap);
+        Map<Expression, Expression> varExprMap = extractLetBindingVariableExpressionMappings(letClauses);
+        return (Expression) SqlppRewriteUtil.substituteExpression(returnExpr, varExprMap, context);
     }
 
     @Override
@@ -249,13 +248,13 @@ public class SqlppInlineUdfsVisitor extends AbstractInlineUdfsVisitor
         return inlined || result.first;
     }
 
-    private Map<VariableExpr, Expression> extractLetBindingVariableExpressionMappings(List<LetClause> letClauses)
+    private Map<Expression, Expression> extractLetBindingVariableExpressionMappings(List<LetClause> letClauses)
             throws AsterixException {
-        Map<VariableExpr, Expression> varExprMap = new HashMap<>();
+        Map<Expression, Expression> varExprMap = new HashMap<>();
         for (LetClause lc : letClauses) {
             // inline let variables one by one iteratively.
-            lc.setBindingExpr((Expression) SqlppVariableSubstitutionUtil
-                    .substituteVariableWithoutContext(lc.getBindingExpr(), varExprMap));
+            lc.setBindingExpr((Expression) SqlppRewriteUtil.substituteExpression(lc.getBindingExpr(),
+                    varExprMap, context));
             varExprMap.put(lc.getVarExpr(), lc.getBindingExpr());
         }
         return varExprMap;

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SubstituteGroupbyExpressionWithVariableVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SubstituteGroupbyExpressionWithVariableVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SubstituteGroupbyExpressionWithVariableVisitor.java
index c5cd60e..eb2f463 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SubstituteGroupbyExpressionWithVariableVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/SubstituteGroupbyExpressionWithVariableVisitor.java
@@ -25,21 +25,25 @@ import java.util.Map;
 import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.common.functions.FunctionSignature;
 import org.apache.asterix.lang.common.base.Expression;
-import org.apache.asterix.lang.common.base.Expression.Kind;
 import org.apache.asterix.lang.common.base.ILangExpression;
+import org.apache.asterix.lang.common.base.Expression.Kind;
+import org.apache.asterix.lang.common.clause.LetClause;
 import org.apache.asterix.lang.common.expression.CallExpr;
 import org.apache.asterix.lang.common.expression.GbyVariableExpressionPair;
-import org.apache.asterix.lang.common.rewrites.ExpressionSubstitutionEnvironment;
+import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
 import org.apache.asterix.lang.sqlpp.clause.SelectBlock;
 import org.apache.asterix.lang.sqlpp.expression.SelectExpression;
 import org.apache.asterix.lang.sqlpp.util.FunctionMapUtil;
-import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil;
-import org.apache.asterix.lang.sqlpp.visitor.SqlppSubstituteExpressionsVisitor;
-import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppSimpleExpressionVisitor;
+import org.apache.asterix.lang.sqlpp.visitor.SqlppSubstituteExpressionVisitor;
+import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScopingVisitor;
 
 // Replaces expressions that appear in having/select/order-by/limit clause and are identical to some
 // group by key expression with the group by key expression.
-public class SubstituteGroupbyExpressionWithVariableVisitor extends AbstractSqlppSimpleExpressionVisitor {
+public class SubstituteGroupbyExpressionWithVariableVisitor extends AbstractSqlppExpressionScopingVisitor {
+
+    public SubstituteGroupbyExpressionWithVariableVisitor(LangRewritingContext context) {
+        super(context);
+    }
 
     @Override
     public Expression visit(SelectBlock selectBlock, ILangExpression arg) throws AsterixException {
@@ -53,21 +57,28 @@ public class SubstituteGroupbyExpressionWithVariableVisitor extends AbstractSqlp
             }
 
             // Creates a substitution visitor.
-            ExpressionSubstitutionEnvironment env =
-                    new ExpressionSubstitutionEnvironment(map, SqlppVariableUtil::getFreeVariables);
-            SubstituteGroupbyExpressionVisitor visitor = new SubstituteGroupbyExpressionVisitor();
+            SubstituteGroupbyExpressionVisitor visitor = new SubstituteGroupbyExpressionVisitor(context, map);
 
-            // Rewrites having/select/order-by/limit clauses.
+            // Rewrites LET/HAVING/SELECT clauses.
+            if(selectBlock.hasLetClausesAfterGroupby()){
+                for(LetClause letClause : selectBlock.getLetListAfterGroupby()){
+                    letClause.accept(this, arg);
+                }
+            }
             if (selectBlock.hasHavingClause()) {
-                selectBlock.getHavingClause().accept(visitor, env);
+                selectBlock.getHavingClause().accept(visitor, arg);
             }
-            selectBlock.getSelectClause().accept(visitor, env);
+            selectBlock.getSelectClause().accept(visitor, arg);
             SelectExpression selectExpression = (SelectExpression) arg;
-            if (selectExpression.hasOrderby()) {
-                selectExpression.getOrderbyClause().accept(visitor, env);
-            }
-            if (selectExpression.hasLimit()) {
-                selectExpression.getLimitClause().accept(visitor, env);
+
+            // For SET operation queries, the GROUP BY key variables will not substitute ORDER BY nor LIMIT expressions.
+            if (!selectExpression.getSelectSetOperation().hasRightInputs()) {
+                if (selectExpression.hasOrderby()) {
+                    selectExpression.getOrderbyClause().accept(visitor, arg);
+                }
+                if (selectExpression.hasLimit()) {
+                    selectExpression.getLimitClause().accept(visitor, arg);
+                }
             }
         }
         return super.visit(selectBlock, arg);
@@ -75,15 +86,19 @@ public class SubstituteGroupbyExpressionWithVariableVisitor extends AbstractSqlp
 
 }
 
-class SubstituteGroupbyExpressionVisitor extends SqlppSubstituteExpressionsVisitor {
+class SubstituteGroupbyExpressionVisitor extends SqlppSubstituteExpressionVisitor {
+
+    public SubstituteGroupbyExpressionVisitor(LangRewritingContext context, Map<Expression, Expression> exprMap) {
+        super(context, exprMap);
+    }
 
     @Override
-    public Expression visit(CallExpr callExpr, ExpressionSubstitutionEnvironment env) throws AsterixException {
+    public Expression visit(CallExpr callExpr, ILangExpression arg) throws AsterixException {
         FunctionSignature signature = callExpr.getFunctionSignature();
         if (FunctionMapUtil.isSql92AggregateFunction(signature)) {
             return callExpr;
         } else {
-            return super.visit(callExpr, env);
+            return super.visit(callExpr, arg);
         }
     }
 }


Mime
View raw message