asterixdb-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From buyin...@apache.org
Subject [2/8] asterixdb git commit: Clean up GROUP BY and WITH clause.
Date Mon, 15 Aug 2016 15:45:33 GMT
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/VariableCheckAndRewriteVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java
index 79c99b8..9769d4f 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java
@@ -26,8 +26,8 @@ import org.apache.asterix.common.config.MetadataConstants;
 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.expression.CallExpr;
 import org.apache.asterix.lang.common.expression.FieldAccessor;
 import org.apache.asterix.lang.common.expression.LiteralExpr;
@@ -102,7 +102,12 @@ public class VariableCheckAndRewriteVisitor extends AbstractSqlppExpressionScopi
         if (!rewriteNeeded(varExpr)) {
             return varExpr;
         }
-        Set<VariableExpr> liveVars = SqlppVariableUtil.getLiveVariables(scopeChecker.getCurrentScope());
+        // Note: WITH variables are not used for path resolution. The reason is that
+        // the accurate typing for ordered list with an UNION item type is not implemented.
+        // We currently type it as [ANY]. If we include WITH variables for path resolution,
+        // it will lead to ambiguities and the plan is going to be very complex.  An example query is:
+        // asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/subquery/exists
+        Set<VariableExpr> liveVars = SqlppVariableUtil.getLiveVariables(scopeChecker.getCurrentScope(), false);
         boolean resolveAsDataset = resolveDatasetFirst(arg) && datasetExists(dataverseName, datasetName);
         if (resolveAsDataset) {
             return wrapWithDatasetFunction(dataverseName, datasetName);

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppRewriteUtil.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppRewriteUtil.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppRewriteUtil.java
index 9bc1813..519963a 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppRewriteUtil.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppRewriteUtil.java
@@ -20,6 +20,7 @@ package org.apache.asterix.lang.sqlpp.util;
 
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.asterix.common.exceptions.AsterixException;
@@ -27,10 +28,12 @@ import org.apache.asterix.lang.common.base.Expression;
 import org.apache.asterix.lang.common.base.ILangExpression;
 import org.apache.asterix.lang.common.expression.VariableExpr;
 import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
+import org.apache.asterix.lang.common.statement.Query;
 import org.apache.asterix.lang.sqlpp.rewrites.visitor.SqlppGroupBySugarVisitor;
 import org.apache.asterix.lang.sqlpp.visitor.CheckSubqueryVisitor;
 import org.apache.asterix.lang.sqlpp.visitor.DeepCopyVisitor;
 import org.apache.asterix.lang.sqlpp.visitor.FreeVariableVisitor;
+import org.apache.asterix.lang.sqlpp.visitor.SqlppSubstituteExpressionVisitor;
 
 public class SqlppRewriteUtil {
 
@@ -39,10 +42,9 @@ public class SqlppRewriteUtil {
 
     // Applying sugar rewriting for group-by.
     public static Expression rewriteExpressionUsingGroupVariable(VariableExpr groupVar,
-            Collection<VariableExpr> targetVarList, Collection<VariableExpr> allVisableVars, ILangExpression expr,
+            Collection<VariableExpr> fieldVars, ILangExpression expr,
             LangRewritingContext context) throws AsterixException {
-        SqlppGroupBySugarVisitor visitor =
-                new SqlppGroupBySugarVisitor(context, groupVar, targetVarList, allVisableVars);
+        SqlppGroupBySugarVisitor visitor = new SqlppGroupBySugarVisitor(context, groupVar, fieldVars);
         return expr.accept(visitor, null);
     }
 
@@ -70,4 +72,31 @@ public class SqlppRewriteUtil {
         return expr.accept(visitor, null);
     }
 
+    /**
+     * Substitutes expression with replacement expressions according to the exprMap.
+     *
+     * @param expression
+     *            ,
+     *            an input expression.
+     * @param exprMap
+     *            a map that maps expressions to their corresponding replacement expressions.
+     * @return an expression, where sub-expressions of the input expression (including the input expression itself)
+     *         are replaced with deep copies with their mapped replacements in the exprMap if there exists such a
+     *         replacement expression.
+     * @throws AsterixException
+     */
+    public static Expression substituteExpression(Expression expression, Map<Expression, Expression> exprMap,
+            LangRewritingContext context) throws AsterixException {
+        if (exprMap.isEmpty()) {
+            return expression;
+        }
+        // Creates a wrapper query for the expression so that if the expression itself
+        // is the key, it can also be replaced.
+        Query wrapper = new Query(false);
+        wrapper.setBody(expression);
+        // Creates a substitution visitor.
+        SqlppSubstituteExpressionVisitor visitor = new SqlppSubstituteExpressionVisitor(context, exprMap);
+        wrapper.accept(visitor, wrapper);
+        return wrapper.getBody();
+    }
 }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableSubstitutionUtil.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableSubstitutionUtil.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableSubstitutionUtil.java
deleted file mode 100644
index 322b5b6..0000000
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableSubstitutionUtil.java
+++ /dev/null
@@ -1,92 +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.sqlpp.util;
-
-import java.util.Map;
-
-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.expression.VariableExpr;
-import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
-import org.apache.asterix.lang.common.rewrites.VariableSubstitutionEnvironment;
-import org.apache.asterix.lang.sqlpp.visitor.SqlppCloneAndSubstituteVariablesVisitor;
-import org.apache.asterix.lang.sqlpp.visitor.SqlppSubstituteVariablesVisitor;
-
-public class SqlppVariableSubstitutionUtil {
-
-    /**
-     * Substitute variables with corresponding expressions according to the varExprMap.
-     * The substitution should be done BEFORE unique ids are assigned to variables.
-     * In other words, when we call this method, ids of all variables are zero.
-     *
-     * @param expression,
-     *            the expression for substituting variables.
-     * @param varExprMap
-     *            a map that maps variables to their corresponding expressions.
-     * @return a new expression in which variables are substituted.
-     * @throws AsterixException
-     */
-    public static ILangExpression substituteVariableWithoutContext(ILangExpression expression,
-            Map<VariableExpr, Expression> varExprMap) throws AsterixException {
-        if (varExprMap.isEmpty()) {
-            return expression;
-        }
-        VariableSubstitutionEnvironment env = new VariableSubstitutionEnvironment(varExprMap);
-        return substituteVariableWithoutContext(expression, env);
-    }
-
-    /**
-     * Substitute variables with corresponding expressions according to the varExprMap.
-     * The substitution should be done BEFORE unique ids are assigned to variables.
-     * In other words, when we call this method, ids of all variables are zero.
-     *
-     * @param expression,
-     *            the expression for substituting variables.
-     * @param env,
-     *            internally contains a map that maps variables to their corresponding expressions.
-     * @return a new expression in which variables are substituted.
-     * @throws AsterixException
-     */
-    public static ILangExpression substituteVariableWithoutContext(ILangExpression expression,
-            VariableSubstitutionEnvironment env) throws AsterixException {
-        SqlppSubstituteVariablesVisitor visitor = new SqlppSubstituteVariablesVisitor();
-        return expression.accept(visitor, env).first;
-    }
-
-    /**
-     * Substitute variables with corresponding expressions according to the varExprMap.
-     * The substitution should be done AFTER unique ids are assigned to different variables.
-     *
-     * @param expression,
-     *            the expression for substituting variables.
-     * @param varExprMap,
-     *            a map that maps variables to their corresponding expressions.
-     * @param context,
-     *            manages the ids of variables so as to guarantee the uniqueness of ids for different variables (even with the same name).
-     * @return a cloned new expression in which variables are substituted, and its bounded variables have new unique ids.
-     * @throws AsterixException
-     */
-    public static ILangExpression cloneAndSubstituteVariable(ILangExpression expression,
-            Map<VariableExpr, Expression> varExprMap, LangRewritingContext context) throws AsterixException {
-        SqlppCloneAndSubstituteVariablesVisitor visitor = new SqlppCloneAndSubstituteVariablesVisitor(context);
-        VariableSubstitutionEnvironment env = new VariableSubstitutionEnvironment(varExprMap);
-        return expression.accept(visitor, env).first;
-    }
-}

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java
index b2a07e1..5352804 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java
@@ -77,12 +77,18 @@ public class SqlppVariableUtil {
         return varName;
     }
 
-    public static Set<VariableExpr> getLiveVariables(Scope scope) {
+    public static Set<VariableExpr> getLiveVariables(Scope scope, boolean includeWithVariables) {
         Set<VariableExpr> results = new HashSet<>();
         Set<VariableExpr> liveVars = scope.getLiveVariables();
         Iterator<VariableExpr> liveVarIter = liveVars.iterator();
         while (liveVarIter.hasNext()) {
-            results.add(liveVarIter.next());
+            VariableExpr liveVar = liveVarIter.next();
+            // Variables defined in WITH clauses are named value access.
+            // TODO(buyingi): remove this if block once we can accurately type
+            // ordered lists with UNION item type. Currently it is typed as [ANY].
+            if (includeWithVariables || !liveVar.getVar().namedValueAccess()) {
+                results.add(liveVar);
+            }
         }
         return results;
     }
@@ -148,7 +154,9 @@ public class SqlppVariableUtil {
                 bindingVars.add(var);
             }
         }
-        bindingVars.addAll(gbyClause.getWithVarList());
+        if (gbyClause.hasWithMap()) {
+            bindingVars.addAll(gbyClause.getWithVarMap().values());
+        }
         bindingVars.add(gbyClause.getGroupVar());
         return bindingVars;
     }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
----------------------------------------------------------------------
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 d752396..75c5307 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
@@ -19,7 +19,10 @@
 package org.apache.asterix.lang.sqlpp.visitor;
 
 import java.util.ArrayList;
+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;
@@ -256,7 +259,7 @@ public class DeepCopyVisitor extends AbstractSqlppQueryExpressionVisitor<ILangEx
     public GroupbyClause visit(GroupbyClause gc, Void arg) throws AsterixException {
         List<GbyVariableExpressionPair> gbyPairList = new ArrayList<>();
         List<GbyVariableExpressionPair> decorPairList = new ArrayList<>();
-        List<VariableExpr> withVarList = new ArrayList<>();
+        Map<Expression, VariableExpr> withVarMap = new HashMap<>();
         VariableExpr groupVarExpr = null;
         List<Pair<Expression, Identifier>> groupFieldList = new ArrayList<>();
         for (GbyVariableExpressionPair gbyVarExpr : gc.getGbyPairList()) {
@@ -269,8 +272,9 @@ public class DeepCopyVisitor extends AbstractSqlppQueryExpressionVisitor<ILangEx
             decorPairList.add(new GbyVariableExpressionPair(var == null ? null : (VariableExpr) var.accept(this, arg),
                     (Expression) gbyVarExpr.getExpr().accept(this, arg)));
         }
-        for (VariableExpr withVar : gc.getWithVarList()) {
-            withVarList.add((VariableExpr) withVar.accept(this, arg));
+        for (Entry<Expression, VariableExpr> entry : gc.getWithVarMap().entrySet()) {
+            withVarMap.put((Expression) entry.getKey().accept(this, arg),
+                    (VariableExpr) entry.getValue().accept(this, arg));
         }
         if (gc.hasGroupVar()) {
             groupVarExpr = (VariableExpr) gc.getGroupVar().accept(this, arg);
@@ -278,7 +282,7 @@ public class DeepCopyVisitor extends AbstractSqlppQueryExpressionVisitor<ILangEx
         for (Pair<Expression, Identifier> field : gc.getGroupFieldList()) {
             groupFieldList.add(new Pair<>((Expression) field.first.accept(this, arg), field.second));
         }
-        return new GroupbyClause(gbyPairList, decorPairList, withVarList, groupVarExpr, groupFieldList,
+        return new GroupbyClause(gbyPairList, decorPairList, withVarMap, groupVarExpr, groupFieldList,
                 gc.hasHashGroupByHint(), gc.isGroupAll());
     }
 
@@ -386,10 +390,8 @@ public class DeepCopyVisitor extends AbstractSqlppQueryExpressionVisitor<ILangEx
 
     @Override
     public VariableExpr visit(VariableExpr varExpr, Void arg) throws AsterixException {
-        VariableExpr clonedVar =
-                new VariableExpr(new VarIdentifier(varExpr.getVar().getValue(), varExpr.getVar().getId()));
+        VariableExpr clonedVar = new VariableExpr(new VarIdentifier(varExpr.getVar()));
         clonedVar.setIsNewVar(varExpr.getIsNewVar());
-        clonedVar.setNamedValueAccess(varExpr.namedValueAccess());
         return clonedVar;
     }
 

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/FreeVariableVisitor.java
----------------------------------------------------------------------
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 20b144c..3a9bebf 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
@@ -23,8 +23,8 @@ import java.util.HashSet;
 import java.util.List;
 
 import org.apache.asterix.common.exceptions.AsterixException;
-import org.apache.asterix.lang.common.base.Clause.ClauseType;
 import org.apache.asterix.lang.common.base.Expression;
+import org.apache.asterix.lang.common.base.Clause.ClauseType;
 import org.apache.asterix.lang.common.clause.GroupbyClause;
 import org.apache.asterix.lang.common.clause.LetClause;
 import org.apache.asterix.lang.common.clause.LimitClause;
@@ -144,7 +144,9 @@ public class FreeVariableVisitor extends AbstractSqlppQueryExpressionVisitor<Voi
 
     @Override
     public Void visit(Projection projection, Collection<VariableExpr> freeVars) throws AsterixException {
-        projection.getExpression().accept(this, freeVars);
+        if (!projection.star()) {
+            projection.getExpression().accept(this, freeVars);
+        }
         return null;
     }
 

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppCloneAndSubstituteVariablesVisitor.java
----------------------------------------------------------------------
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 23181e4..81aba54 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
@@ -22,9 +22,9 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.asterix.common.exceptions.AsterixException;
-import org.apache.asterix.lang.common.base.Clause.ClauseType;
 import org.apache.asterix.lang.common.base.Expression;
 import org.apache.asterix.lang.common.base.ILangExpression;
+import org.apache.asterix.lang.common.base.Clause.ClauseType;
 import org.apache.asterix.lang.common.clause.GroupbyClause;
 import org.apache.asterix.lang.common.clause.LetClause;
 import org.apache.asterix.lang.common.clause.LimitClause;
@@ -86,8 +86,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             VariableSubstitutionEnvironment env) throws AsterixException {
         VariableExpr leftVar = fromTerm.getLeftVariable();
         VariableExpr newLeftVar = generateNewVariable(context, leftVar);
-        VariableExpr newLeftPosVar = fromTerm.hasPositionalVariable()
-                ? generateNewVariable(context, fromTerm.getPositionalVariable()) : null;
+        VariableExpr newLeftPosVar = fromTerm.hasPositionalVariable() ? generateNewVariable(context,
+                fromTerm.getPositionalVariable()) : null;
         Expression newLeftExpr = (Expression) visitUnnesBindingExpression(fromTerm.getLeftExpression(), env).first;
         List<AbstractBinaryCorrelateClause> newCorrelateClauses = new ArrayList<>();
 
@@ -108,6 +108,11 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
                 // The right-hand-side of join and nest could not be correlated with the left side,
                 // therefore we propagate the original substitution environment.
                 newCorrelateClauses.add((AbstractBinaryCorrelateClause) correlateClause.accept(this, env).first);
+                // Join binding variables should be removed for further traversal.
+                currentEnv.removeSubstitution(correlateClause.getRightVariable());
+                if (correlateClause.hasPositionalVariable()) {
+                    currentEnv.removeSubstitution(correlateClause.getPositionalVariable());
+                }
             }
         }
         return new Pair<>(new FromTerm(newLeftExpr, newLeftVar, newLeftPosVar, newCorrelateClauses), currentEnv);
@@ -118,8 +123,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             VariableSubstitutionEnvironment env) throws AsterixException {
         VariableExpr rightVar = joinClause.getRightVariable();
         VariableExpr newRightVar = generateNewVariable(context, rightVar);
-        VariableExpr newRightPosVar = joinClause.hasPositionalVariable()
-                ? generateNewVariable(context, joinClause.getPositionalVariable()) : null;
+        VariableExpr newRightPosVar = joinClause.hasPositionalVariable() ? generateNewVariable(context,
+                joinClause.getPositionalVariable()) : null;
 
         // Visits the right expression.
         Expression newRightExpr = (Expression) visitUnnesBindingExpression(joinClause.getRightExpression(), env).first;
@@ -133,8 +138,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
         // The condition can refer to the newRightVar and newRightPosVar.
         Expression conditionExpr = (Expression) joinClause.getConditionExpression().accept(this, currentEnv).first;
 
-        JoinClause newJoinClause =
-                new JoinClause(joinClause.getJoinType(), newRightExpr, newRightVar, newRightPosVar, conditionExpr);
+        JoinClause newJoinClause = new JoinClause(joinClause.getJoinType(), newRightExpr, newRightVar, newRightPosVar,
+                conditionExpr);
         return new Pair<>(newJoinClause, currentEnv);
     }
 
@@ -143,8 +148,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             VariableSubstitutionEnvironment env) throws AsterixException {
         VariableExpr rightVar = nestClause.getRightVariable();
         VariableExpr newRightVar = generateNewVariable(context, rightVar);
-        VariableExpr newRightPosVar = nestClause.hasPositionalVariable()
-                ? generateNewVariable(context, nestClause.getPositionalVariable()) : null;
+        VariableExpr newRightPosVar = nestClause.hasPositionalVariable() ? generateNewVariable(context,
+                nestClause.getPositionalVariable()) : null;
 
         // Visits the right expression.
         Expression rightExpr = (Expression) nestClause.getRightExpression().accept(this, env).first;
@@ -158,8 +163,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
         // The condition can refer to the newRightVar and newRightPosVar.
         Expression conditionExpr = (Expression) nestClause.getConditionExpression().accept(this, currentEnv).first;
 
-        NestClause newJoinClause =
-                new NestClause(nestClause.getJoinType(), rightExpr, newRightVar, newRightPosVar, conditionExpr);
+        NestClause newJoinClause = new NestClause(nestClause.getJoinType(), rightExpr, newRightVar, newRightPosVar,
+                conditionExpr);
         return new Pair<>(newJoinClause, currentEnv);
     }
 
@@ -168,8 +173,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             VariableSubstitutionEnvironment env) throws AsterixException {
         VariableExpr rightVar = unnestClause.getRightVariable();
         VariableExpr newRightVar = generateNewVariable(context, rightVar);
-        VariableExpr newRightPosVar = unnestClause.hasPositionalVariable()
-                ? generateNewVariable(context, unnestClause.getPositionalVariable()) : null;
+        VariableExpr newRightPosVar = unnestClause.hasPositionalVariable() ? generateNewVariable(context,
+                unnestClause.getPositionalVariable()) : null;
 
         // Visits the right expression.
         Expression rightExpr = (Expression) visitUnnesBindingExpression(unnestClause.getRightExpression(), env).first;
@@ -181,8 +186,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             currentEnv.removeSubstitution(newRightPosVar);
         }
         // The condition can refer to the newRightVar and newRightPosVar.
-        UnnestClause newJoinClause =
-                new UnnestClause(unnestClause.getJoinType(), rightExpr, newRightVar, newRightPosVar);
+        UnnestClause newJoinClause = new UnnestClause(unnestClause.getJoinType(), rightExpr, newRightVar,
+                newRightPosVar);
         return new Pair<>(newJoinClause, currentEnv);
     }
 
@@ -217,7 +222,7 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
 
         if (selectBlock.hasLetClauses()) {
             for (LetClause letClause : selectBlock.getLetList()) {
-                newLet = letClause.accept(this, env);
+                newLet = letClause.accept(this, currentEnv);
                 currentEnv = newLet.second;
                 newLetClauses.add(letClause);
             }
@@ -233,7 +238,7 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             currentEnv = newGroupby.second;
             if (selectBlock.hasLetClausesAfterGroupby()) {
                 for (LetClause letClauseAfterGby : selectBlock.getLetListAfterGroupby()) {
-                    newLet = letClauseAfterGby.accept(this, env);
+                    newLet = letClauseAfterGby.accept(this, currentEnv);
                     currentEnv = newLet.second;
                     newLetClausesAfterGby.add(letClauseAfterGby);
                 }
@@ -260,13 +265,13 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
             VariableSubstitutionEnvironment env) throws AsterixException {
         boolean distinct = selectClause.distinct();
         if (selectClause.selectElement()) {
-            Pair<ILangExpression, VariableSubstitutionEnvironment> newSelectElement =
-                    selectClause.getSelectElement().accept(this, env);
+            Pair<ILangExpression, VariableSubstitutionEnvironment> newSelectElement = selectClause.getSelectElement()
+                    .accept(this, env);
             return new Pair<>(new SelectClause((SelectElement) newSelectElement.first, null, distinct),
                     newSelectElement.second);
         } else {
-            Pair<ILangExpression, VariableSubstitutionEnvironment> newSelectRegular =
-                    selectClause.getSelectRegular().accept(this, env);
+            Pair<ILangExpression, VariableSubstitutionEnvironment> newSelectRegular = selectClause.getSelectRegular()
+                    .accept(this, env);
             return new Pair<>(new SelectClause(null, (SelectRegular) newSelectRegular.first, distinct),
                     newSelectRegular.second);
         }
@@ -275,8 +280,8 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
     @Override
     public Pair<ILangExpression, VariableSubstitutionEnvironment> visit(SelectElement selectElement,
             VariableSubstitutionEnvironment env) throws AsterixException {
-        Pair<ILangExpression, VariableSubstitutionEnvironment> newExpr =
-                selectElement.getExpression().accept(this, env);
+        Pair<ILangExpression, VariableSubstitutionEnvironment> newExpr = selectElement.getExpression()
+                .accept(this, env);
         return new Pair<>(new SelectElement((Expression) newExpr.first), newExpr.second);
     }
 
@@ -296,32 +301,36 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
         SetOperationInput leftInput = selectSetOperation.getLeftInput();
         SetOperationInput newLeftInput;
 
+        Pair<ILangExpression, VariableSubstitutionEnvironment> leftResult;
         // Sets the left input.
         if (leftInput.selectBlock()) {
-            Pair<ILangExpression, VariableSubstitutionEnvironment> p = leftInput.getSelectBlock().accept(this, env);
-            newLeftInput = new SetOperationInput((SelectBlock) p.first, null);
+            leftResult = leftInput.getSelectBlock().accept(this, env);
+            newLeftInput = new SetOperationInput((SelectBlock) leftResult.first, null);
         } else {
-            Pair<ILangExpression, VariableSubstitutionEnvironment> p = leftInput.getSubquery().accept(this, env);
-            newLeftInput = new SetOperationInput(null, (SelectExpression) p.first);
+            leftResult = leftInput.getSubquery().accept(this, env);
+            newLeftInput = new SetOperationInput(null, (SelectExpression) leftResult.first);
         }
 
         // Sets the right input
         List<SetOperationRight> newRightInputs = new ArrayList<>();
-        for (SetOperationRight right : selectSetOperation.getRightInputs()) {
-            SetOperationInput newRightInput;
-            SetOperationInput rightInput = right.getSetOperationRightInput();
-            if (rightInput.selectBlock()) {
-                Pair<ILangExpression, VariableSubstitutionEnvironment> p =
-                        rightInput.getSelectBlock().accept(this, env);
-                newRightInput = new SetOperationInput((SelectBlock) p.first, null);
-            } else {
-                Pair<ILangExpression, VariableSubstitutionEnvironment> p = rightInput.getSubquery().accept(this, env);
-                newRightInput = new SetOperationInput(null, (SelectExpression) p.first);
+        if (selectSetOperation.hasRightInputs()) {
+            for (SetOperationRight right : selectSetOperation.getRightInputs()) {
+                SetOperationInput newRightInput;
+                SetOperationInput rightInput = right.getSetOperationRightInput();
+                if (rightInput.selectBlock()) {
+                    Pair<ILangExpression, VariableSubstitutionEnvironment> rightResult = rightInput.getSelectBlock()
+                            .accept(this, env);
+                    newRightInput = new SetOperationInput((SelectBlock) rightResult.first, null);
+                } else {
+                    Pair<ILangExpression, VariableSubstitutionEnvironment> rightResult = rightInput.getSubquery()
+                            .accept(this, env);
+                    newRightInput = new SetOperationInput(null, (SelectExpression) rightResult.first);
+                }
+                newRightInputs.add(new SetOperationRight(right.getSetOpType(), right.isSetSemantics(), newRightInput));
             }
-            newRightInputs.add(new SetOperationRight(right.getSetOpType(), right.isSetSemantics(), newRightInput));
         }
         SelectSetOperation newSelectSetOperation = new SelectSetOperation(newLeftInput, newRightInputs);
-        return new Pair<>(newSelectSetOperation, env);
+        return new Pair<>(newSelectSetOperation, selectSetOperation.hasRightInputs() ? env : leftResult.second);
     }
 
     @Override
@@ -348,13 +357,13 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
         currentEnv = p.second;
 
         if (selectExpression.hasOrderby()) {
-            p = selectExpression.getOrderbyClause().accept(this, env);
+            p = selectExpression.getOrderbyClause().accept(this, currentEnv);
             newOrderbyClause = (OrderbyClause) p.first;
             currentEnv = p.second;
         }
 
         if (selectExpression.hasLimit()) {
-            p = selectExpression.getLimitClause().accept(this, env);
+            p = selectExpression.getLimitClause().accept(this, currentEnv);
             newLimitClause = (LimitClause) p.first;
             currentEnv = p.second;
         }
@@ -383,10 +392,10 @@ public class SqlppCloneAndSubstituteVariablesVisitor extends CloneAndSubstituteV
     public Pair<ILangExpression, VariableSubstitutionEnvironment> visit(CaseExpression caseExpr,
             VariableSubstitutionEnvironment env) throws AsterixException {
         Expression conditionExpr = (Expression) caseExpr.getConditionExpr().accept(this, env).first;
-        List<Expression> whenExprList =
-                VariableCloneAndSubstitutionUtil.visitAndCloneExprList(caseExpr.getWhenExprs(), env, this);
-        List<Expression> thenExprList =
-                VariableCloneAndSubstitutionUtil.visitAndCloneExprList(caseExpr.getThenExprs(), env, this);
+        List<Expression> whenExprList = VariableCloneAndSubstitutionUtil.visitAndCloneExprList(caseExpr.getWhenExprs(),
+                env, this);
+        List<Expression> thenExprList = VariableCloneAndSubstitutionUtil.visitAndCloneExprList(caseExpr.getThenExprs(),
+                env, this);
         Expression elseExpr = (Expression) caseExpr.getElseExpr().accept(this, env).first;
         CaseExpression newCaseExpr = new CaseExpression(conditionExpr, whenExprList, thenExprList, elseExpr);
         return new Pair<>(newCaseExpr, env);

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionVisitor.java
new file mode 100644
index 0000000..9b19d7c
--- /dev/null
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionVisitor.java
@@ -0,0 +1,69 @@
+/*
+ * 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.sqlpp.visitor;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.asterix.common.exceptions.AsterixException;
+import org.apache.asterix.lang.common.base.Expression;
+import org.apache.asterix.lang.common.context.Scope;
+import org.apache.asterix.lang.common.expression.VariableExpr;
+import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
+import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
+import org.apache.asterix.lang.sqlpp.util.SqlppVariableUtil;
+import org.apache.asterix.lang.sqlpp.visitor.base.AbstractSqlppExpressionScopingVisitor;
+
+/**
+ * This visitor is used to substitute expressions in a visiting language structure
+ * with an given map.
+ */
+public class SqlppSubstituteExpressionVisitor extends AbstractSqlppExpressionScopingVisitor {
+
+    private final Map<Expression, Expression> exprMap = new HashMap<>();
+
+    public SqlppSubstituteExpressionVisitor(LangRewritingContext context, Map<Expression, Expression> exprMap) {
+        super(context);
+        this.exprMap.putAll(exprMap);
+    }
+
+    // Note: we intentionally override preVisit instead of postVisit because we wants larger expressions
+    // get substituted first if some of their child expressions are present as keys in the exprMap.
+    // An example is:
+    // asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/group-by/gby-expr-3/gby-expr-3.3.query.sqlpp
+    @Override
+    protected Expression preVisit(Expression expr) throws AsterixException {
+        Expression mappedExpr = exprMap.get(expr);
+        if (mappedExpr == null) {
+            return expr;
+        }
+        Collection<VariableExpr> freeVars = SqlppVariableUtil.getFreeVariables(expr);
+        for (VariableExpr freeVar : freeVars) {
+            Scope currentScope = scopeChecker.getCurrentScope();
+            if (currentScope.findSymbol(freeVar.getVar().getValue()) != null) {
+                // If the expression to be substituted uses variables defined in the outer-most expresion
+                // that is being visited, we shouldn't perform the substitution.
+                return expr;
+            }
+        }
+        // Makes a deep copy before returning to avoid shared references.
+        return (Expression) SqlppRewriteUtil.deepCopy(mappedExpr);
+    }
+}

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionsVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionsVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionsVisitor.java
deleted file mode 100644
index d9b2698..0000000
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionsVisitor.java
+++ /dev/null
@@ -1,287 +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.sqlpp.visitor;
-
-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.GroupbyClause;
-import org.apache.asterix.lang.common.clause.LetClause;
-import org.apache.asterix.lang.common.expression.GbyVariableExpressionPair;
-import org.apache.asterix.lang.common.rewrites.ExpressionSubstitutionEnvironment;
-import org.apache.asterix.lang.common.visitor.SubstituteExpressionVisitor;
-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.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.visitor.base.ISqlppVisitor;
-
-public class SqlppSubstituteExpressionsVisitor extends SubstituteExpressionVisitor
-        implements ISqlppVisitor<Expression, ExpressionSubstitutionEnvironment> {
-
-    public SqlppSubstituteExpressionsVisitor() {
-        super(SqlppRewriteUtil::deepCopy);
-    }
-
-    @Override
-    public Expression visit(GroupbyClause gc, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        for (GbyVariableExpressionPair pair : gc.getGbyPairList()) {
-            pair.setExpr(pair.getExpr().accept(this, env));
-        }
-        // Forces from binding variables to exit their scopes, i.e.,
-        // one can still replace from binding variables.
-        env.pop();
-        for (GbyVariableExpressionPair pair : gc.getGbyPairList()) {
-            env.disableVariable(pair.getVar());
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(FromClause fromClause, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        // Marks the states before the from clause, and a consequent
-        // group-by clause will reset env to the state.
-        env.mark();
-        for (FromTerm fromTerm : fromClause.getFromTerms()) {
-            fromTerm.accept(this, env);
-            // From terms are correlated and thus we mask binding variables after
-            // visiting each individual from term.
-            env.disableVariable(fromTerm.getLeftVariable());
-            if (fromTerm.hasPositionalVariable()) {
-                env.disableVariable(fromTerm.getLeftVariable());
-            }
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(FromTerm fromTerm, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        fromTerm.setLeftExpression(fromTerm.getLeftExpression().accept(this, env));
-        if (fromTerm.hasCorrelateClauses()) {
-            for (AbstractBinaryCorrelateClause correlateClause : fromTerm.getCorrelateClauses()) {
-                correlateClause.accept(this, env);
-            }
-            // correlate clauses are independent and thus we mask their binding variables
-            // after visiting them.
-            for (AbstractBinaryCorrelateClause correlateClause : fromTerm.getCorrelateClauses()) {
-                env.disableVariable(correlateClause.getRightVariable());
-                if (correlateClause.hasPositionalVariable()) {
-                    env.disableVariable(correlateClause.getPositionalVariable());
-                }
-            }
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(JoinClause joinClause, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        joinClause.setRightExpression(joinClause.getRightExpression().accept(this, env));
-        // Condition expressions can see the join binding variables, thus we have to mask them for replacement.
-        env.disableVariable(joinClause.getRightVariable());
-        if (joinClause.hasPositionalVariable()) {
-            env.disableVariable(joinClause.getPositionalVariable());
-        }
-        joinClause.setConditionExpression(joinClause.getConditionExpression().accept(this, env));
-        // Re-enable them.
-        env.enableVariable(joinClause.getRightVariable());
-        if (joinClause.hasPositionalVariable()) {
-            env.enableVariable(joinClause.getPositionalVariable());
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(NestClause nestClause, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        nestClause.setRightExpression(nestClause.getRightExpression().accept(this, env));
-        // Condition expressions can see the join binding variables, thus we have to mask them for replacement.
-        env.disableVariable(nestClause.getRightVariable());
-        if (nestClause.hasPositionalVariable()) {
-            env.disableVariable(nestClause.getPositionalVariable());
-        }
-        nestClause.setConditionExpression(nestClause.getConditionExpression().accept(this, env));
-        // Re-enable them.
-        env.enableVariable(nestClause.getRightVariable());
-        if (nestClause.hasPositionalVariable()) {
-            env.enableVariable(nestClause.getPositionalVariable());
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(Projection projection, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        if (!projection.star()) {
-            projection.setExpression(projection.getExpression().accept(this, env));
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(SelectBlock selectBlock, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        // Traverses the select block in the order of "from", "let"s, "where",
-        // "group by", "let"s, "having" and "select".
-        if (selectBlock.hasFromClause()) {
-            selectBlock.getFromClause().accept(this, env);
-        }
-        if (selectBlock.hasLetClauses()) {
-            List<LetClause> letList = selectBlock.getLetList();
-            for (LetClause letClause : letList) {
-                letClause.accept(this, env);
-            }
-        }
-        if (selectBlock.hasWhereClause()) {
-            selectBlock.getWhereClause().accept(this, env);
-        }
-        if (selectBlock.hasGroupbyClause()) {
-            selectBlock.getGroupbyClause().accept(this, env);
-        }
-        if (selectBlock.hasLetClausesAfterGroupby()) {
-            List<LetClause> letListAfterGby = selectBlock.getLetListAfterGroupby();
-            for (LetClause letClauseAfterGby : letListAfterGby) {
-                letClauseAfterGby.accept(this, env);
-            }
-        }
-        if (selectBlock.hasHavingClause()) {
-            selectBlock.getHavingClause().accept(this, env);
-        }
-        selectBlock.getSelectClause().accept(this, env);
-        return null;
-    }
-
-    @Override
-    public Expression visit(SelectClause selectClause, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        if (selectClause.selectElement()) {
-            selectClause.getSelectElement().accept(this, env);
-        } else {
-            selectClause.getSelectRegular().accept(this, env);
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(SelectElement selectElement, ExpressionSubstitutionEnvironment env)
-            throws AsterixException {
-        selectElement.setExpression(selectElement.getExpression().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(SelectRegular selectRegular, ExpressionSubstitutionEnvironment env)
-            throws AsterixException {
-        for (Projection projection : selectRegular.getProjections()) {
-            projection.accept(this, env);
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(SelectSetOperation selectSetOperation, ExpressionSubstitutionEnvironment env)
-            throws AsterixException {
-        selectSetOperation.getLeftInput().accept(this, env);
-        for (SetOperationRight right : selectSetOperation.getRightInputs()) {
-            right.getSetOperationRightInput().accept(this, env);
-        }
-        return null;
-    }
-
-    @Override
-    public Expression visit(SelectExpression selectExpression, ExpressionSubstitutionEnvironment env)
-            throws AsterixException {
-        // Backups the current states of env and the end of this method will reset to the returned depth.
-        int depth = env.mark();
-        // visit let list
-        if (selectExpression.hasLetClauses()) {
-            for (LetClause letClause : selectExpression.getLetList()) {
-                letClause.accept(this, env);
-            }
-        }
-
-        // visit the main select.
-        selectExpression.getSelectSetOperation().accept(this, env);
-
-        // visit order by.
-        if (selectExpression.hasOrderby()) {
-            selectExpression.getOrderbyClause().accept(this, env);
-        }
-
-        // visit limit
-        if (selectExpression.hasLimit()) {
-            selectExpression.getLimitClause().accept(this, env);
-        }
-
-        // re-enable the replacements all of all binding variables in the current scope.
-        if (selectExpression.hasLetClauses()) {
-            for (LetClause letClause : selectExpression.getLetList()) {
-                env.enableVariable(letClause.getVarExpr());
-            }
-        }
-        // Restores the states of env to be the same as the beginning .
-        env.reset(depth);
-        return selectExpression;
-    }
-
-    @Override
-    public Expression visit(UnnestClause unnestClause, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        unnestClause.setRightExpression(unnestClause.getRightExpression().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(HavingClause havingClause, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        havingClause.setFilterExpression(havingClause.getFilterExpression().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(IndependentSubquery independentSubquery, ExpressionSubstitutionEnvironment env)
-            throws AsterixException {
-        independentSubquery.setExpr(independentSubquery.getExpr().accept(this, env));
-        return null;
-    }
-
-    @Override
-    public Expression visit(CaseExpression caseExpr, ExpressionSubstitutionEnvironment env) throws AsterixException {
-        Expression newCaseExpr = env.findSubstitution(caseExpr, deepCopier);
-        if (newCaseExpr == caseExpr) {
-            caseExpr.setConditionExpr(caseExpr.getConditionExpr().accept(this, env));
-            caseExpr.setWhenExprs(rewriteExpressionList(caseExpr.getWhenExprs(), env));
-            caseExpr.setThenExprs(rewriteExpressionList(caseExpr.getThenExprs(), env));
-            caseExpr.setElseExpr(caseExpr.getElseExpr().accept(this, env));
-            return caseExpr;
-        } else {
-            return newCaseExpr.accept(this, env);
-        }
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteVariablesVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteVariablesVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteVariablesVisitor.java
deleted file mode 100644
index 923f86d..0000000
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteVariablesVisitor.java
+++ /dev/null
@@ -1,48 +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.sqlpp.visitor;
-
-import org.apache.asterix.common.exceptions.AsterixException;
-import org.apache.asterix.lang.common.base.Expression;
-import org.apache.asterix.lang.common.expression.VariableExpr;
-import org.apache.asterix.lang.common.rewrites.LangRewritingContext;
-import org.apache.asterix.lang.common.rewrites.VariableSubstitutionEnvironment;
-import org.apache.asterix.lang.sqlpp.util.SqlppRewriteUtil;
-
-public class SqlppSubstituteVariablesVisitor extends SqlppCloneAndSubstituteVariablesVisitor {
-
-    public SqlppSubstituteVariablesVisitor() {
-        super(null);
-    }
-
-    @Override
-    protected Expression rewriteVariableExpr(VariableExpr expr, VariableSubstitutionEnvironment env)
-            throws AsterixException {
-        if (env.constainsOldVar(expr)) {
-            return (Expression) SqlppRewriteUtil.deepCopy(env.findSubstitution(expr));
-        }
-        return expr;
-    }
-
-    @Override
-    public VariableExpr generateNewVariable(LangRewritingContext context, VariableExpr varExpr) {
-        return varExpr;
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java
index 13ec855..c13f00d 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppExpressionScopingVisitor.java
@@ -19,7 +19,10 @@
 package org.apache.asterix.lang.sqlpp.visitor.base;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import org.apache.asterix.common.config.MetadataConstants;
@@ -55,6 +58,7 @@ 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.SqlppVariableUtil;
+import org.apache.hyracks.algebricks.common.utils.Pair;
 import org.apache.hyracks.algebricks.core.algebra.base.Counter;
 import org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier;
 
@@ -87,7 +91,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
     public Expression visit(FromTerm fromTerm, ILangExpression arg) throws AsterixException {
         scopeChecker.createNewScope();
         // Visit the left expression of a from term.
-        fromTerm.setLeftExpression(fromTerm.getLeftExpression().accept(this, fromTerm));
+        fromTerm.setLeftExpression(visit(fromTerm.getLeftExpression(), fromTerm));
 
         // Registers the data item variable.
         VariableExpr leftVar = fromTerm.getLeftVariable();
@@ -113,7 +117,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
         // NOTE: the two join branches cannot be correlated, instead of checking
         // the correlation here,
         // we defer the check to the query optimizer.
-        joinClause.setRightExpression(joinClause.getRightExpression().accept(this, joinClause));
+        joinClause.setRightExpression(visit(joinClause.getRightExpression(), joinClause));
 
         // Registers the data item variable.
         VariableExpr rightVar = joinClause.getRightVariable();
@@ -132,7 +136,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
         scopeChecker.pushExistingScope(mergedScope);
         // The condition expression can refer to the just registered variables
         // for the right branch.
-        joinClause.setConditionExpression(joinClause.getConditionExpression().accept(this, joinClause));
+        joinClause.setConditionExpression(visit(joinClause.getConditionExpression(), joinClause));
         return null;
     }
 
@@ -141,7 +145,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
         // NOTE: the two branches of a NEST cannot be correlated, instead of
         // checking the correlation here, we defer the check to the query
         // optimizer.
-        nestClause.setRightExpression(nestClause.getRightExpression().accept(this, nestClause));
+        nestClause.setRightExpression(visit(nestClause.getRightExpression(), nestClause));
 
         // Registers the data item variable.
         VariableExpr rightVar = nestClause.getRightVariable();
@@ -155,13 +159,13 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
 
         // The condition expression can refer to the just registered variables
         // for the right branch.
-        nestClause.setConditionExpression(nestClause.getConditionExpression().accept(this, nestClause));
+        nestClause.setConditionExpression(visit(nestClause.getConditionExpression(), nestClause));
         return null;
     }
 
     @Override
     public Expression visit(UnnestClause unnestClause, ILangExpression arg) throws AsterixException {
-        unnestClause.setRightExpression(unnestClause.getRightExpression().accept(this, unnestClause));
+        unnestClause.setRightExpression(visit(unnestClause.getRightExpression(), unnestClause));
 
         // register the data item variable
         VariableExpr rightVar = unnestClause.getRightVariable();
@@ -199,7 +203,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
 
     @Override
     public Expression visit(Query q, ILangExpression arg) throws AsterixException {
-        q.setBody(q.getBody().accept(this, q));
+        q.setBody(visit(q.getBody(), q));
         q.setVarCounter(scopeChecker.getVarCounter());
         context.setVarCounter(scopeChecker.getVarCounter());
         return null;
@@ -208,25 +212,53 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
     @Override
     public Expression visit(FunctionDecl fd, ILangExpression arg) throws AsterixException {
         scopeChecker.createNewScope();
-        fd.setFuncBody(fd.getFuncBody().accept(this, fd));
+        fd.setFuncBody(visit(fd.getFuncBody(), fd));
         scopeChecker.removeCurrentScope();
         return null;
     }
 
     @Override
     public Expression visit(GroupbyClause gc, ILangExpression arg) throws AsterixException {
-        Scope newScope = scopeChecker.extendCurrentScopeNoPush(true);
+        // After a GROUP BY, variables defined before the current SELECT BLOCK (e.g., in a WITH clause
+        // or an outer scope query) should still be visible.
+        Scope newScope = new Scope(scopeChecker, scopeChecker.getCurrentScope().getParentScope());
         // Puts all group-by variables into the symbol set of the new scope.
-        for (GbyVariableExpressionPair gbyVarExpr : gc.getGbyPairList()) {
-            gbyVarExpr.setExpr(gbyVarExpr.getExpr().accept(this, gc));
-            VariableExpr gbyVar = gbyVarExpr.getVar();
-            if (gbyVar != null) {
-                newScope.addNewVarSymbolToScope(gbyVarExpr.getVar().getVar());
+        for (GbyVariableExpressionPair gbyKeyVarExpr : gc.getGbyPairList()) {
+            gbyKeyVarExpr.setExpr(visit(gbyKeyVarExpr.getExpr(), gc));
+            VariableExpr gbyKeyVar = gbyKeyVarExpr.getVar();
+            if (gbyKeyVar != null) {
+                newScope.addNewVarSymbolToScope(gbyKeyVar.getVar());
             }
         }
-        for (VariableExpr withVar : gc.getWithVarList()) {
-            newScope.addNewVarSymbolToScope(withVar.getVar());
+        if (gc.hasGroupFieldList()) {
+            for (Pair<Expression, Identifier> gbyField : gc.getGroupFieldList()) {
+                gbyField.first = visit(gbyField.first, arg);
+            }
+        }
+        if (gc.hasDecorList()) {
+            for (GbyVariableExpressionPair decorVarExpr : gc.getDecorPairList()) {
+                decorVarExpr.setExpr(visit(decorVarExpr.getExpr(), gc));
+                VariableExpr decorVar = decorVarExpr.getVar();
+                if (decorVar != null) {
+                    newScope.addNewVarSymbolToScope(decorVar.getVar());
+                }
+            }
         }
+        if (gc.hasGroupVar()) {
+            scopeChecker.getCurrentScope().addNewVarSymbolToScope(gc.getGroupVar().getVar());
+        }
+        if (gc.hasWithMap()) {
+            Map<Expression, VariableExpr> newWithMap = new HashMap<>();
+            for (Entry<Expression, VariableExpr> entry : gc.getWithVarMap().entrySet()) {
+                Expression expr = visit(entry.getKey(), arg);
+                Expression newKey = expr;
+                VariableExpr value = entry.getValue();
+                newScope.addNewVarSymbolToScope(value.getVar());
+                newWithMap.put(newKey, value);
+            }
+            gc.setWithVarMap(newWithMap);
+        }
+        // Replaces the current scope with the new scope.
         scopeChecker.replaceCurrentScope(newScope);
         return null;
     }
@@ -234,7 +266,10 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
     @Override
     public Expression visit(LimitClause limitClause, ILangExpression arg) throws AsterixException {
         scopeChecker.pushForbiddenScope(scopeChecker.getCurrentScope());
-        limitClause.setLimitExpr(limitClause.getLimitExpr().accept(this, limitClause));
+        limitClause.setLimitExpr(visit(limitClause.getLimitExpr(), limitClause));
+        if(limitClause.hasOffset()) {
+            limitClause.setOffset(visit(limitClause.getOffset(), limitClause));
+        }
         scopeChecker.popForbiddenScope();
         return null;
     }
@@ -242,7 +277,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
     @Override
     public Expression visit(LetClause letClause, ILangExpression arg) throws AsterixException {
         scopeChecker.extendCurrentScope();
-        letClause.setBindingExpr(letClause.getBindingExpr().accept(this, letClause));
+        letClause.setBindingExpr(visit(letClause.getBindingExpr(), letClause));
         scopeChecker.getCurrentScope().addNewVarSymbolToScope(letClause.getVarExpr().getVar());
         return null;
     }
@@ -255,8 +290,11 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
         // visit let list
         if (selectExpression.hasLetClauses()) {
             for (LetClause letClause : selectExpression.getLetList()) {
+                // Variables defined in WITH clauses are considered as named access instead of real variables.
+                letClause.getVarExpr().getVar().setNamedValueAccess(true);
                 letClause.accept(this, selectExpression);
             }
+            scopeChecker.createNewScope();
         }
 
         // visit the main select.
@@ -286,7 +324,7 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
         // variables defined in the parent scope.
         Scope scope = new Scope(scopeChecker, scopeChecker.getCurrentScope(), true);
         scopeChecker.pushExistingScope(scope);
-        independentSubquery.setExpr(independentSubquery.getExpr().accept(this, independentSubquery));
+        independentSubquery.setExpr(visit(independentSubquery.getExpr(), independentSubquery));
         scopeChecker.removeCurrentScope();
         return independentSubquery;
     }
@@ -295,10 +333,10 @@ public class AbstractSqlppExpressionScopingVisitor extends AbstractSqlppSimpleEx
     public Expression visit(QuantifiedExpression qe, ILangExpression arg) throws AsterixException {
         scopeChecker.createNewScope();
         for (QuantifiedPair pair : qe.getQuantifiedList()) {
-            pair.setExpr(pair.getExpr().accept(this, qe));
+            pair.setExpr(visit(pair.getExpr(), qe));
             scopeChecker.getCurrentScope().addNewVarSymbolToScope(pair.getVarExpr().getVar());
         }
-        qe.setSatisfiesExpr(qe.getSatisfiesExpr().accept(this, qe));
+        qe.setSatisfiesExpr(visit(qe.getSatisfiesExpr(), qe));
         scopeChecker.removeCurrentScope();
         return qe;
     }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/AbstractSqlppSimpleExpressionVisitor.java
----------------------------------------------------------------------
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 423eb40..21883f7 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
@@ -21,6 +21,7 @@ package org.apache.asterix.lang.sqlpp.visitor.base;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.antlr.stringtemplate.language.Expr;
 import org.apache.asterix.common.exceptions.AsterixException;
 import org.apache.asterix.lang.common.base.Expression;
 import org.apache.asterix.lang.common.base.ILangExpression;
@@ -69,7 +70,7 @@ public class AbstractSqlppSimpleExpressionVisitor
     @Override
     public Expression visit(FromClause fromClause, ILangExpression arg) throws AsterixException {
         for (FromTerm fromTerm : fromClause.getFromTerms()) {
-            fromTerm.accept(this, arg);
+            visit(fromTerm, arg);
         }
         return null;
     }
@@ -77,7 +78,7 @@ public class AbstractSqlppSimpleExpressionVisitor
     @Override
     public Expression visit(FromTerm fromTerm, ILangExpression arg) throws AsterixException {
         // Visit the left expression of a from term.
-        fromTerm.setLeftExpression(fromTerm.getLeftExpression().accept(this, arg));
+        fromTerm.setLeftExpression(visit(fromTerm.getLeftExpression(), arg));
 
         // Visits join/unnest/nest clauses.
         for (AbstractBinaryCorrelateClause correlateClause : fromTerm.getCorrelateClauses()) {
@@ -88,28 +89,28 @@ public class AbstractSqlppSimpleExpressionVisitor
 
     @Override
     public Expression visit(JoinClause joinClause, ILangExpression arg) throws AsterixException {
-        joinClause.setRightExpression(joinClause.getRightExpression().accept(this, arg));
-        joinClause.setConditionExpression(joinClause.getConditionExpression().accept(this, arg));
+        joinClause.setRightExpression(visit(joinClause.getRightExpression(), arg));
+        joinClause.setConditionExpression(visit(joinClause.getConditionExpression(), arg));
         return null;
     }
 
     @Override
     public Expression visit(NestClause nestClause, ILangExpression arg) throws AsterixException {
-        nestClause.setRightExpression(nestClause.getRightExpression().accept(this, arg));
-        nestClause.setConditionExpression(nestClause.getConditionExpression().accept(this, arg));
+        nestClause.setRightExpression(visit(nestClause.getRightExpression(), arg));
+        nestClause.setConditionExpression(visit(nestClause.getConditionExpression(), arg));
         return null;
     }
 
     @Override
     public Expression visit(UnnestClause unnestClause, ILangExpression arg) throws AsterixException {
-        unnestClause.setRightExpression(unnestClause.getRightExpression().accept(this, arg));
+        unnestClause.setRightExpression(visit(unnestClause.getRightExpression(), arg));
         return null;
     }
 
     @Override
     public Expression visit(Projection projection, ILangExpression arg) throws AsterixException {
         if (!projection.star()) {
-            projection.setExpression(projection.getExpression().accept(this, arg));
+            projection.setExpression(visit(projection.getExpression(), arg));
         }
         return null;
     }
@@ -159,7 +160,7 @@ public class AbstractSqlppSimpleExpressionVisitor
 
     @Override
     public Expression visit(SelectElement selectElement, ILangExpression arg) throws AsterixException {
-        selectElement.setExpression(selectElement.getExpression().accept(this, selectElement));
+        selectElement.setExpression(visit(selectElement.getExpression(), selectElement));
         return null;
     }
 
@@ -182,58 +183,54 @@ public class AbstractSqlppSimpleExpressionVisitor
 
     @Override
     public Expression visit(HavingClause havingClause, ILangExpression arg) throws AsterixException {
-        havingClause.setFilterExpression(havingClause.getFilterExpression().accept(this, havingClause));
+        havingClause.setFilterExpression(visit(havingClause.getFilterExpression(), havingClause));
         return null;
     }
 
     @Override
     public Expression visit(Query q, ILangExpression arg) throws AsterixException {
-        q.setBody(q.getBody().accept(this, q));
+        q.setBody(visit(q.getBody(), q));
         return null;
     }
 
     @Override
     public Expression visit(FunctionDecl fd, ILangExpression arg) throws AsterixException {
-        fd.setFuncBody(fd.getFuncBody().accept(this, fd));
+        fd.setFuncBody(visit(fd.getFuncBody(), fd));
         return null;
     }
 
     @Override
     public Expression visit(WhereClause whereClause, ILangExpression arg) throws AsterixException {
-        whereClause.setWhereExpr(whereClause.getWhereExpr().accept(this, whereClause));
+        whereClause.setWhereExpr(visit(whereClause.getWhereExpr(), whereClause));
         return null;
     }
 
     @Override
     public Expression visit(OrderbyClause oc, ILangExpression arg) throws AsterixException {
-        List<Expression> newOrderbyList = new ArrayList<>();
-        for (Expression orderExpr : oc.getOrderbyList()) {
-            newOrderbyList.add(orderExpr.accept(this, oc));
-        }
-        oc.setOrderbyList(newOrderbyList);
+        oc.setOrderbyList(visit(oc.getOrderbyList(), arg));
         return null;
     }
 
     @Override
     public Expression visit(GroupbyClause gc, ILangExpression arg) throws AsterixException {
         for (GbyVariableExpressionPair gbyVarExpr : gc.getGbyPairList()) {
-            gbyVarExpr.setExpr(gbyVarExpr.getExpr().accept(this, gc));
+            gbyVarExpr.setExpr(visit(gbyVarExpr.getExpr(), gc));
         }
         return null;
     }
 
     @Override
     public Expression visit(LimitClause limitClause, ILangExpression arg) throws AsterixException {
-        limitClause.setLimitExpr(limitClause.getLimitExpr().accept(this, limitClause));
+        limitClause.setLimitExpr(visit(limitClause.getLimitExpr(), limitClause));
         if (limitClause.hasOffset()) {
-            limitClause.setOffset(limitClause.getOffset().accept(this, limitClause));
+            limitClause.setOffset(visit(limitClause.getOffset(), limitClause));
         }
         return null;
     }
 
     @Override
     public Expression visit(LetClause letClause, ILangExpression arg) throws AsterixException {
-        letClause.setBindingExpr(letClause.getBindingExpr().accept(this, letClause));
+        letClause.setBindingExpr(visit(letClause.getBindingExpr(), letClause));
         return null;
     }
 
@@ -275,8 +272,8 @@ public class AbstractSqlppSimpleExpressionVisitor
     @Override
     public Expression visit(RecordConstructor rc, ILangExpression arg) throws AsterixException {
         for (FieldBinding binding : rc.getFbList()) {
-            binding.setLeftExpr(binding.getLeftExpr().accept(this, rc));
-            binding.setRightExpr(binding.getRightExpr().accept(this, rc));
+            binding.setLeftExpr(visit(binding.getLeftExpr(), rc));
+            binding.setRightExpr(visit(binding.getRightExpr(), rc));
         }
         return rc;
     }
@@ -289,28 +286,24 @@ public class AbstractSqlppSimpleExpressionVisitor
 
     @Override
     public Expression visit(IfExpr ifExpr, ILangExpression arg) throws AsterixException {
-        ifExpr.setCondExpr(ifExpr.getCondExpr().accept(this, ifExpr));
-        ifExpr.setThenExpr(ifExpr.getThenExpr().accept(this, ifExpr));
-        ifExpr.setElseExpr(ifExpr.getElseExpr().accept(this, ifExpr));
+        ifExpr.setCondExpr(visit(ifExpr.getCondExpr(), ifExpr));
+        ifExpr.setThenExpr(visit(ifExpr.getThenExpr(), ifExpr));
+        ifExpr.setElseExpr(visit(ifExpr.getElseExpr(), ifExpr));
         return ifExpr;
     }
 
     @Override
     public Expression visit(QuantifiedExpression qe, ILangExpression arg) throws AsterixException {
         for (QuantifiedPair pair : qe.getQuantifiedList()) {
-            pair.setExpr(pair.getExpr().accept(this, qe));
+            pair.setExpr(visit(pair.getExpr(), qe));
         }
-        qe.setSatisfiesExpr(qe.getSatisfiesExpr().accept(this, qe));
+        qe.setSatisfiesExpr(visit(qe.getSatisfiesExpr(), qe));
         return qe;
     }
 
     @Override
     public Expression visit(CallExpr callExpr, ILangExpression arg) throws AsterixException {
-        List<Expression> newExprList = new ArrayList<>();
-        for (Expression expr : callExpr.getExprList()) {
-            newExprList.add(expr.accept(this, callExpr));
-        }
-        callExpr.setExprList(newExprList);
+        callExpr.setExprList(visit(callExpr.getExprList(), arg));
         return callExpr;
     }
 
@@ -321,46 +314,57 @@ public class AbstractSqlppSimpleExpressionVisitor
 
     @Override
     public Expression visit(UnaryExpr u, ILangExpression arg) throws AsterixException {
-        u.setExpr(u.getExpr().accept(this, u));
+        u.setExpr(visit(u.getExpr(), u));
         return u;
     }
 
     @Override
     public Expression visit(FieldAccessor fa, ILangExpression arg) throws AsterixException {
-        fa.setExpr(fa.getExpr().accept(this, fa));
+        fa.setExpr(visit(fa.getExpr(), fa));
         return fa;
     }
 
     @Override
     public Expression visit(IndexAccessor ia, ILangExpression arg) throws AsterixException {
-        ia.setExpr(ia.getExpr().accept(this, ia));
+        ia.setExpr(visit(ia.getExpr(), ia));
         if (ia.getIndexExpr() != null) {
-            ia.setIndexExpr(ia.getIndexExpr());
+            ia.setIndexExpr(visit(ia.getIndexExpr(), arg));
         }
         return ia;
     }
 
     @Override
     public Expression visit(IndependentSubquery independentSubquery, ILangExpression arg) throws AsterixException {
-        independentSubquery.setExpr(independentSubquery.getExpr().accept(this, arg));
+        independentSubquery.setExpr(visit(independentSubquery.getExpr(), arg));
         return independentSubquery;
     }
 
     @Override
     public Expression visit(CaseExpression caseExpr, ILangExpression arg) throws AsterixException {
-        caseExpr.setConditionExpr(caseExpr.getConditionExpr().accept(this, arg));
+        caseExpr.setConditionExpr(visit(caseExpr.getConditionExpr(), arg));
         caseExpr.setWhenExprs(visit(caseExpr.getWhenExprs(), arg));
         caseExpr.setThenExprs(visit(caseExpr.getThenExprs(), arg));
-        caseExpr.setElseExpr(caseExpr.getElseExpr().accept(this, arg));
+        caseExpr.setElseExpr(visit(caseExpr.getElseExpr(), arg));
         return caseExpr;
     }
 
+    protected Expression visit(Expression expr, ILangExpression arg) throws AsterixException{
+        return postVisit(preVisit(expr).accept(this, arg));
+    }
+
+    protected Expression preVisit(Expression expr) throws AsterixException{
+        return expr;
+    }
+
+    protected Expression postVisit(Expression expr) throws AsterixException {
+        return expr;
+    }
+
     private List<Expression> visit(List<Expression> exprs, ILangExpression arg) throws AsterixException {
         List<Expression> newExprList = new ArrayList<>();
         for (Expression expr : exprs) {
-            newExprList.add(expr.accept(this, arg));
+            newExprList.add(visit(expr, arg));
         }
         return newExprList;
     }
-
 }

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index a0a9742..95f09f5 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
+++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
@@ -2649,7 +2649,6 @@ GroupbyClause GroupbyClause()throws ParseException :
     GroupbyClause gbc = new GroupbyClause();
     List<GbyVariableExpressionPair> vePairList = new ArrayList<GbyVariableExpressionPair>();
     VariableExpr var = null;
-    VariableExpr withVar = null;
     Expression expr = null;
     VariableExpr decorVar = null;
     Expression decorExpr = null;
@@ -2721,7 +2720,7 @@ GroupbyClause GroupbyClause()throws ParseException :
     {
       gbc.setGbyPairList(vePairList);
       gbc.setDecorPairList(new ArrayList<GbyVariableExpressionPair>());
-      gbc.setWithVarList(new ArrayList<VariableExpr>());
+      gbc.setWithVarMap(new HashMap<Expression, VariableExpr>());
       gbc.setGroupVar(groupVar);
       gbc.setGroupFieldList(groupFieldList);
       replaceCurrentScope(newScope);

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java
index e303d97..97f0ace 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java
@@ -83,6 +83,7 @@ import org.apache.asterix.om.typecomputer.impl.OrderedListOfAIntervalTypeCompute
 import org.apache.asterix.om.typecomputer.impl.OrderedListOfAPointTypeComputer;
 import org.apache.asterix.om.typecomputer.impl.OrderedListOfAStringTypeComputer;
 import org.apache.asterix.om.typecomputer.impl.OrderedListOfAnyTypeComputer;
+import org.apache.asterix.om.typecomputer.impl.PropagateTypeComputer;
 import org.apache.asterix.om.typecomputer.impl.RecordAddFieldsTypeComputer;
 import org.apache.asterix.om.typecomputer.impl.RecordMergeTypeComputer;
 import org.apache.asterix.om.typecomputer.impl.RecordRemoveFieldsTypeComputer;
@@ -301,6 +302,10 @@ public class AsterixBuiltinFunctions {
             "agg-intermediate-avg", 1);
     public static final FunctionIdentifier LOCAL_AVG = new FunctionIdentifier(FunctionConstants.ASTERIX_NS,
             "agg-local-avg", 1);
+    public static final FunctionIdentifier FIRST_ELEMENT = new FunctionIdentifier(FunctionConstants.ASTERIX_NS,
+            "agg-first-element", 1);
+    public static final FunctionIdentifier LOCAL_FIRST_ELEMENT = new FunctionIdentifier(FunctionConstants.ASTERIX_NS,
+            "agg-local-first-element", 1);
 
     public static final FunctionIdentifier SCALAR_AVG = new FunctionIdentifier(FunctionConstants.ASTERIX_NS, "avg", 1);
     public static final FunctionIdentifier SCALAR_COUNT = new FunctionIdentifier(FunctionConstants.ASTERIX_NS, "count",
@@ -312,6 +317,8 @@ public class AsterixBuiltinFunctions {
             "global-avg", 1);
     public static final FunctionIdentifier SCALAR_LOCAL_AVG = new FunctionIdentifier(FunctionConstants.ASTERIX_NS,
             "local-avg", 1);
+    public static final FunctionIdentifier SCALAR_FIRST_ELEMENT = new FunctionIdentifier(FunctionConstants.ASTERIX_NS,
+            "first-element", 1);
 
     // serializable aggregate functions
     public static final FunctionIdentifier SERIAL_AVG = new FunctionIdentifier(FunctionConstants.ASTERIX_NS,
@@ -864,6 +871,9 @@ public class AsterixBuiltinFunctions {
         addFunction(SUM, NumericAggTypeComputer.INSTANCE, true);
         addPrivateFunction(LOCAL_SUM, NumericAggTypeComputer.INSTANCE, true);
         addPrivateFunction(GLOBAL_AVG, NullableDoubleTypeComputer.INSTANCE, true);
+        addPrivateFunction(SCALAR_FIRST_ELEMENT, CollectionMemberResultType.INSTANCE, true);
+        addPrivateFunction(FIRST_ELEMENT, PropagateTypeComputer.INSTANCE, true);
+        addPrivateFunction(LOCAL_FIRST_ELEMENT, PropagateTypeComputer.INSTANCE, true);
 
         addPrivateFunction(SERIAL_SQL_AVG, NullableDoubleTypeComputer.INSTANCE, true);
         addPrivateFunction(SERIAL_SQL_COUNT, AInt64TypeComputer.INSTANCE, true);
@@ -1067,6 +1077,8 @@ public class AsterixBuiltinFunctions {
         scalarToAggregateFunctionMap.put(getAsterixFunctionInfo(SCALAR_MAX), getAsterixFunctionInfo(MAX));
         scalarToAggregateFunctionMap.put(getAsterixFunctionInfo(SCALAR_MIN), getAsterixFunctionInfo(MIN));
         scalarToAggregateFunctionMap.put(getAsterixFunctionInfo(SCALAR_SUM), getAsterixFunctionInfo(SUM));
+        scalarToAggregateFunctionMap.put(getAsterixFunctionInfo(SCALAR_FIRST_ELEMENT),
+                getAsterixFunctionInfo(FIRST_ELEMENT));
         // SQL Aggregate Functions
         scalarToAggregateFunctionMap.put(getAsterixFunctionInfo(SCALAR_SQL_AVG), getAsterixFunctionInfo(SQL_AVG));
         scalarToAggregateFunctionMap.put(getAsterixFunctionInfo(SCALAR_SQL_COUNT), getAsterixFunctionInfo(SQL_COUNT));
@@ -1101,6 +1113,13 @@ public class AsterixBuiltinFunctions {
         addIntermediateAgg(MAX, MAX);
         addGlobalAgg(MAX, MAX);
 
+        addAgg(SCALAR_FIRST_ELEMENT);
+        addAgg(LOCAL_FIRST_ELEMENT);
+        addLocalAgg(FIRST_ELEMENT, LOCAL_FIRST_ELEMENT);
+        addIntermediateAgg(LOCAL_FIRST_ELEMENT, FIRST_ELEMENT);
+        addIntermediateAgg(FIRST_ELEMENT, FIRST_ELEMENT);
+        addGlobalAgg(FIRST_ELEMENT, FIRST_ELEMENT);
+
         addAgg(MIN);
         addLocalAgg(MIN, LOCAL_MIN);
         addIntermediateAgg(LOCAL_MIN, MIN);

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java
index 4332bb1..2931d7e 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java
@@ -42,10 +42,12 @@ public class FieldAccessByNameResultType extends AbstractResultTypeComputer {
     @Override
     protected void checkArgType(int argIndex, IAType type) throws AlgebricksException {
         if (argIndex == 0 && type.getTypeTag() != ATypeTag.RECORD) {
-            throw new AlgebricksException("The first argument should be a RECORD, but it is " + type + ".");
+            throw new AlgebricksException("The first argument of a field access should be a RECORD, but it is " + type
+                    + ".");
         }
         if (argIndex == 1 && type.getTypeTag() != ATypeTag.STRING) {
-            throw new AlgebricksException("The second argument should be an STRING, but it is found " + type + ".");
+            throw new AlgebricksException("The second argument of a field access should be an STRING, but it is "
+                    + type + ".");
         }
     }
 

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/PropagateTypeComputer.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/PropagateTypeComputer.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/PropagateTypeComputer.java
new file mode 100644
index 0000000..df4524f
--- /dev/null
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/PropagateTypeComputer.java
@@ -0,0 +1,37 @@
+/*
+ * 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.om.typecomputer.impl;
+
+import org.apache.asterix.om.typecomputer.base.AbstractResultTypeComputer;
+import org.apache.asterix.om.types.IAType;
+import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException;
+import org.apache.hyracks.algebricks.core.algebra.base.ILogicalExpression;
+
+/**
+ * This type computer just populates whatever input type as the output type.
+ */
+public class PropagateTypeComputer extends AbstractResultTypeComputer {
+
+    public static final PropagateTypeComputer INSTANCE = new PropagateTypeComputer();
+
+    @Override
+    public IAType getResultType(ILogicalExpression expr, IAType... knownTypes) throws AlgebricksException {
+        return knownTypes[0];
+    }
+}

http://git-wip-us.apache.org/repos/asf/asterixdb/blob/8671ddf8/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeHelper.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeHelper.java b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeHelper.java
index 4839c41..588b7d0 100644
--- a/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeHelper.java
+++ b/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/TypeHelper.java
@@ -50,10 +50,7 @@ public class TypeHelper {
             case ANY:
                 return false;
             case UNION:
-                if (!isClosed(((AUnionType) t).getActualType())) {
-                    return false;
-                }
-                return true;
+                return isClosed(((AUnionType) t).getActualType());
             default:
                 return true;
         }


Mime
View raw message