You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by mb...@apache.org on 2020/08/28 20:29:13 UTC

[asterixdb] branch master updated (dcb4ad2 -> 798123d)

This is an automated email from the ASF dual-hosted git repository.

mblow pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git.


    from dcb4ad2  [NO ISSUE][COMP] CREATE OR REPLACE FUNCTION
     new 6dc926a  [NO ISSUE][COMP] Eliminate OR if argument is TRUE/FALSE
     new e08456c  [NO ISSUE][COMP] Fix reference sharing in some optimizer rules
     new 37d339e  [NO ISSUE][COMP] Support skip-index for IN operator
     new 406b304  [ASTERIXDB-2756][REP] Skip Replication of Dropped Indexes
     new 0e432a0  [NO ISSUE][RT] Fix upserting into secondary indexes
     new 798123d  Merge branch 'gerrit/mad-hatter'

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../optimizer/rules/ConstantFoldingRule.java       | 60 +++++++++++++++++++---
 .../rules/DisjunctivePredicateToJoinRule.java      | 24 +++++++--
 .../IntroduceSecondaryIndexInsertDeleteRule.java   | 37 ++++++++-----
 .../optimizer/rules/am/BTreeAccessMethod.java      |  3 ++
 .../translator/LangExpressionToPlanTranslator.java | 45 ++++++++--------
 .../SqlppExpressionToPlanTranslator.java           | 26 ++++++----
 .../skip-index/skip-secondary-btree-index-3.sqlpp  | 43 ++++++++++++++++
 .../statement-params-index-01.sqlpp                | 54 +++++++++++++++++++
 .../statement-params-index-02.sqlpp                | 54 +++++++++++++++++++
 .../skip-index/skip-secondary-btree-index-3.plan   | 15 ++++++
 .../statement-params-index-01.plan                 | 19 +++++++
 .../statement-params-index-02.plan                 | 30 +++++++++++
 .../constant_folding.2.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.3.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.4.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.5.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.6.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.7.query.sqlpp                 | 26 ++++++++++
 .../statement-params/index_01/index_01.1.ddl.sqlpp | 41 +++++++++++++++
 .../index_01/index_01.2.update.sqlpp               | 26 ++++++++++
 .../index_01/index_01.3.query.sqlpp                | 36 +++++++++++++
 .../index_01/index_01.4.query.sqlpp                | 36 +++++++++++++
 .../misc/constant_folding/constant_folding.2.adm   |  4 ++
 .../misc/constant_folding/constant_folding.3.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.4.adm   |  4 ++
 .../misc/constant_folding/constant_folding.5.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.6.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.7.adm   | 18 +++++++
 .../statement-params/index_01/index_01.3.adm       |  8 +++
 .../statement-params/index_01/index_01.4.adm       |  1 +
 .../test/resources/runtimets/testsuite_sqlpp.xml   |  5 ++
 .../common/storage/DatasetResourceReference.java   |  2 +
 .../asterix/common/storage/IndexCheckpoint.java    |  2 +-
 .../visitor/OperatorExpressionVisitor.java         |  1 +
 .../management/IndexReplicationManager.java        | 11 ++--
 .../LSMSecondaryUpsertOperatorNodePushable.java    | 41 +++++++++------
 .../PersistentLocalResourceRepository.java         |  5 +-
 .../algebra/util/OperatorManipulationUtil.java     | 12 +++++
 .../AbstractIntroduceGroupByCombinerRule.java      |  2 +-
 .../rules/EnforceStructuralPropertiesRule.java     |  5 +-
 .../rules/ExtractCommonExpressionsRule.java        |  1 +
 .../rewriter/rules/ExtractCommonOperatorsRule.java | 17 +++---
 .../rewriter/rules/IntroduceProjectsRule.java      |  1 +
 .../rules/subplan/PushSubplanIntoGroupByRule.java  | 13 +++--
 44 files changed, 821 insertions(+), 91 deletions(-)
 create mode 100644 asterixdb/asterix-app/src/test/resources/optimizerts/queries/skip-index/skip-secondary-btree-index-3.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-01.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-02.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/optimizerts/results/skip-index/skip-secondary-btree-index-3.plan
 create mode 100644 asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-01.plan
 create mode 100644 asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-02.plan
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.2.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.3.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.4.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.5.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.6.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.7.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.1.ddl.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.2.update.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.3.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.4.query.sqlpp
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.2.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.3.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.4.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.5.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.6.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.7.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.3.adm
 create mode 100644 asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.4.adm


[asterixdb] 03/06: [NO ISSUE][COMP] Support skip-index for IN operator

Posted by mb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit 37d339e25636de8eaa5ad7f21bbf18c3873219b4
Author: Dmitry Lychagin <dm...@couchbase.com>
AuthorDate: Mon Aug 17 17:20:53 2020 -0700

    [NO ISSUE][COMP] Support skip-index for IN operator
    
    Details:
    - Support skip-index hint for IN operator:
      WHERE indexed_field /*+ skip-index */ IN [...]
    
    Change-Id: Ib76f43b53c2bc1b0f5c12e03f555a8d2c1d75701
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/7603
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Dmitry Lychagin <dm...@couchbase.com>
    Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
 .../rules/DisjunctivePredicateToJoinRule.java      | 24 ++++++++++--
 .../SqlppExpressionToPlanTranslator.java           | 26 ++++++++-----
 .../skip-index/skip-secondary-btree-index-3.sqlpp  | 43 ++++++++++++++++++++++
 .../skip-index/skip-secondary-btree-index-3.plan   | 15 ++++++++
 .../visitor/OperatorExpressionVisitor.java         |  1 +
 5 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java
index 916fd75..fe03d75 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java
@@ -18,9 +18,13 @@
  */
 package org.apache.asterix.optimizer.rules;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 
+import org.apache.asterix.common.annotations.SkipSecondaryIndexSearchExpressionAnnotation;
 import org.apache.asterix.metadata.declared.MetadataProvider;
 import org.apache.asterix.om.base.AOrderedList;
 import org.apache.asterix.om.constants.AsterixConstantValue;
@@ -39,6 +43,7 @@ import org.apache.hyracks.algebricks.core.algebra.base.LogicalVariable;
 import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.BroadcastExpressionAnnotation;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ConstantExpression;
+import org.apache.hyracks.algebricks.core.algebra.expressions.IExpressionAnnotation;
 import org.apache.hyracks.algebricks.core.algebra.expressions.IndexedNLJoinExpressionAnnotation;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ScalarFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.UnnestingFunctionCallExpression;
@@ -89,6 +94,7 @@ public class DisjunctivePredicateToJoinRule implements IAlgebraicRewriteRule {
         VariableReferenceExpression varEx = null;
         IAType valType = null;
         HashSet<AsterixConstantValue> values = new HashSet<AsterixConstantValue>();
+        Map<Object, IExpressionAnnotation> allAnnotations = Collections.emptyMap();
 
         for (Mutable<ILogicalExpression> arg : args) {
             AbstractFunctionCallExpression fctCall;
@@ -128,6 +134,12 @@ public class DisjunctivePredicateToJoinRule implements IAlgebraicRewriteRule {
             if (!(haveVar && haveConst)) {
                 return false;
             }
+            if (!fctCall.getAnnotations().isEmpty()) {
+                if (allAnnotations.isEmpty()) {
+                    allAnnotations = new HashMap<>();
+                }
+                allAnnotations.putAll(fctCall.getAnnotations());
+            }
         }
 
         SourceLocation sourceLoc = select.getSourceLocation();
@@ -142,8 +154,9 @@ public class DisjunctivePredicateToJoinRule implements IAlgebraicRewriteRule {
 
         ILogicalExpression cExp = new ConstantExpression(new AsterixConstantValue(list));
         Mutable<ILogicalExpression> mutCExp = new MutableObject<>(cExp);
-        IFunctionInfo scanFctInfo = BuiltinFunctions.getAsterixFunctionInfo(BuiltinFunctions.SCAN_COLLECTION);
-        UnnestingFunctionCallExpression scanExp = new UnnestingFunctionCallExpression(scanFctInfo, mutCExp);
+        UnnestingFunctionCallExpression scanExp = new UnnestingFunctionCallExpression(
+                BuiltinFunctions.getAsterixFunctionInfo(BuiltinFunctions.SCAN_COLLECTION));
+        scanExp.getArguments().add(mutCExp);
         scanExp.setSourceLocation(sourceLoc);
         LogicalVariable scanVar = context.newVar();
         UnnestOperator unn = new UnnestOperator(scanVar, new MutableObject<>(scanExp));
@@ -158,11 +171,14 @@ public class DisjunctivePredicateToJoinRule implements IAlgebraicRewriteRule {
         scanVarRef.setSourceLocation(sourceLoc);
         eqExp.getArguments().add(new MutableObject<>(scanVarRef));
         eqExp.getArguments().add(new MutableObject<>(varEx.cloneExpression()));
-        eqExp.getAnnotations().put(IndexedNLJoinExpressionAnnotation.INSTANCE,
-                IndexedNLJoinExpressionAnnotation.INSTANCE);
+        if (!allAnnotations.containsKey(SkipSecondaryIndexSearchExpressionAnnotation.INSTANCE)) {
+            eqExp.getAnnotations().put(IndexedNLJoinExpressionAnnotation.INSTANCE,
+                    IndexedNLJoinExpressionAnnotation.INSTANCE);
+        }
         BroadcastExpressionAnnotation bcast = new BroadcastExpressionAnnotation();
         bcast.setObject(BroadcastExpressionAnnotation.BroadcastSide.LEFT); // Broadcast the OR predicates branch.
         eqExp.getAnnotations().put(bcast, bcast);
+        eqExp.getAnnotations().putAll(allAnnotations);
 
         InnerJoinOperator jOp = new InnerJoinOperator(new MutableObject<>(eqExp));
         jOp.setSourceLocation(sourceLoc);
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java
index 5363ae2..2cd402f 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/SqlppExpressionToPlanTranslator.java
@@ -109,6 +109,7 @@ import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractFunctionCa
 import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractLogicalExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.AggregateFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ConstantExpression;
+import org.apache.hyracks.algebricks.core.algebra.expressions.IExpressionAnnotation;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ScalarFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.UnnestingFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.VariableReferenceExpression;
@@ -978,7 +979,9 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla
 
         QuantifiedPair qp = qe.getQuantifiedList().get(0);
         VariableExpr varExpr = qp.getVarExpr();
-        List<Expression> operandExprs = ((OperatorExpr) qe.getSatisfiesExpr()).getExprList();
+        OperatorExpr condExpr = (OperatorExpr) qe.getSatisfiesExpr();
+        List<IExpressionAnnotation> condExprHints = condExpr.getHints();
+        List<Expression> operandExprs = condExpr.getExprList();
         int varIdx = operandExprs.indexOf(varExpr);
         Expression operandExpr = operandExprs.get(1 - varIdx);
 
@@ -1011,7 +1014,7 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla
                             throw new CompilationException(ErrorCode.COMPILATION_ILLEGAL_STATE, sourceLoc,
                                     itemExpr.getKind());
                     }
-                    ILogicalExpression eqExpr = createEqExpr(operandVar, inValue, sourceLoc);
+                    ILogicalExpression eqExpr = createEqExpr(operandVar, inValue, condExprHints, sourceLoc);
                     disjuncts.add(new MutableObject<>(eqExpr));
                 }
                 break;
@@ -1022,7 +1025,7 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla
                 inVarCursor.reset();
                 while (inVarCursor.next()) {
                     IAObject inValue = inVarCursor.get();
-                    ILogicalExpression eqExpr = createEqExpr(operandVar, inValue, sourceLoc);
+                    ILogicalExpression eqExpr = createEqExpr(operandVar, inValue, condExprHints, sourceLoc);
                     disjuncts.add(new MutableObject<>(eqExpr));
                 }
                 break;
@@ -1030,25 +1033,25 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla
                 throw new IllegalStateException(String.valueOf(inExpr.getKind()));
         }
 
-        MutableObject<ILogicalExpression> condExpr;
+        MutableObject<ILogicalExpression> disjunctiveExpr;
         if (disjuncts.size() == 1) {
-            condExpr = disjuncts.get(0);
+            disjunctiveExpr = disjuncts.get(0);
         } else {
             AbstractFunctionCallExpression orExpr =
                     createFunctionCallExpressionForBuiltinOperator(OperatorType.OR, sourceLoc);
             orExpr.getArguments().addAll(disjuncts);
-            condExpr = new MutableObject<>(orExpr);
+            disjunctiveExpr = new MutableObject<>(orExpr);
         }
 
         LogicalVariable assignVar = context.newVar();
-        AssignOperator assignOp = new AssignOperator(assignVar, condExpr);
+        AssignOperator assignOp = new AssignOperator(assignVar, disjunctiveExpr);
         assignOp.getInputs().add(topOp);
         assignOp.setSourceLocation(sourceLoc);
         return new Pair<>(assignOp, assignVar);
     }
 
-    private ILogicalExpression createEqExpr(LogicalVariable lhsVar, IAObject rhsValue, SourceLocation sourceLoc)
-            throws CompilationException {
+    private ILogicalExpression createEqExpr(LogicalVariable lhsVar, IAObject rhsValue,
+            List<IExpressionAnnotation> hints, SourceLocation sourceLoc) throws CompilationException {
         VariableReferenceExpression lhsExpr = new VariableReferenceExpression(lhsVar);
         lhsExpr.setSourceLocation(sourceLoc);
         ILogicalExpression rhsExpr = translateConstantValue(rhsValue, sourceLoc);
@@ -1056,6 +1059,11 @@ public class SqlppExpressionToPlanTranslator extends LangExpressionToPlanTransla
                 createFunctionCallExpressionForBuiltinOperator(OperatorType.EQ, sourceLoc);
         opExpr.getArguments().add(new MutableObject<>(lhsExpr));
         opExpr.getArguments().add(new MutableObject<>(rhsExpr));
+        if (hints != null) {
+            for (IExpressionAnnotation hint : hints) {
+                opExpr.getAnnotations().put(hint, hint);
+            }
+        }
         return opExpr;
     }
 
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/skip-index/skip-secondary-btree-index-3.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/skip-index/skip-secondary-btree-index-3.sqlpp
new file mode 100644
index 0000000..d634a26
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/skip-index/skip-secondary-btree-index-3.sqlpp
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ */
+/*
+ * Description  : Skip secondary index for IN operator
+ * Expected Res : Success
+ */
+
+drop  dataverse test if exists;
+create  dataverse test;
+
+use test;
+
+create type test.TestType as
+{
+  id : integer,
+  fname : string,
+  lname : string
+};
+
+create  dataset testdst(TestType) primary key id;
+
+create  index sec_Idx  on testdst (fname) type btree;
+
+select element emp
+from  testdst as emp
+where emp.fname /*+ skip-index */ in ['Max', 'Roger']
+;
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/skip-index/skip-secondary-btree-index-3.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/skip-index/skip-secondary-btree-index-3.plan
new file mode 100644
index 0000000..87bb65f
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/skip-index/skip-secondary-btree-index-3.plan
@@ -0,0 +1,15 @@
+-- DISTRIBUTE_RESULT  |PARTITIONED|
+  -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+    -- STREAM_PROJECT  |PARTITIONED|
+      -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+        -- HYBRID_HASH_JOIN [$$14][$$17]  |PARTITIONED|
+          -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+            -- ASSIGN  |PARTITIONED|
+              -- STREAM_PROJECT  |PARTITIONED|
+                -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                  -- DATASOURCE_SCAN  |PARTITIONED|
+                    -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                      -- EMPTY_TUPLE_SOURCE  |PARTITIONED|
+          -- BROADCAST_EXCHANGE  |PARTITIONED|
+            -- UNNEST  |UNPARTITIONED|
+              -- EMPTY_TUPLE_SOURCE  |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/OperatorExpressionVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/OperatorExpressionVisitor.java
index 23b4d60..9c49071 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/OperatorExpressionVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/OperatorExpressionVisitor.java
@@ -103,6 +103,7 @@ public class OperatorExpressionVisitor extends AbstractSqlppExpressionScopingVis
         comparison.addOperand(itemExpr);
         comparison.addOperand(bindingVar);
         comparison.setCurrentop(true);
+        comparison.addHints(operatorExpr.getHints());
         comparison.setSourceLocation(operatorExpr.getSourceLocation());
         if (opType == OperatorType.IN) {
             comparison.addOperator(OperatorType.EQ);


[asterixdb] 01/06: [NO ISSUE][COMP] Eliminate OR if argument is TRUE/FALSE

Posted by mb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit 6dc926a670be8fe0ebaf9f72ca9416cd4aabfbde
Author: Dmitry Lychagin <dm...@couchbase.com>
AuthorDate: Wed May 27 18:58:03 2020 -0700

    [NO ISSUE][COMP] Eliminate OR if argument is TRUE/FALSE
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    - Eliminate disjunction if one argument is TRUE or FALSE
      as follows: OR(TRUE,x) -> TRUE; OR(FALSE,x) -> x
    
    Change-Id: I896c323da437bdbc822a8484b9981ae909a4b805
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/6523
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
 .../optimizer/rules/ConstantFoldingRule.java       | 60 +++++++++++++++++++---
 .../statement-params-index-01.sqlpp                | 54 +++++++++++++++++++
 .../statement-params-index-02.sqlpp                | 54 +++++++++++++++++++
 .../statement-params-index-01.plan                 | 19 +++++++
 .../statement-params-index-02.plan                 | 30 +++++++++++
 .../constant_folding.2.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.3.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.4.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.5.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.6.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.7.query.sqlpp                 | 26 ++++++++++
 .../statement-params/index_01/index_01.1.ddl.sqlpp | 41 +++++++++++++++
 .../index_01/index_01.2.update.sqlpp               | 26 ++++++++++
 .../index_01/index_01.3.query.sqlpp                | 36 +++++++++++++
 .../index_01/index_01.4.query.sqlpp                | 36 +++++++++++++
 .../misc/constant_folding/constant_folding.2.adm   |  4 ++
 .../misc/constant_folding/constant_folding.3.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.4.adm   |  4 ++
 .../misc/constant_folding/constant_folding.5.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.6.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.7.adm   | 18 +++++++
 .../statement-params/index_01/index_01.3.adm       |  8 +++
 .../statement-params/index_01/index_01.4.adm       |  1 +
 .../test/resources/runtimets/testsuite_sqlpp.xml   |  5 ++
 24 files changed, 600 insertions(+), 6 deletions(-)

diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
index 494b1b4..47e0373 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
@@ -70,6 +70,7 @@ import org.apache.hyracks.algebricks.core.algebra.expressions.AbstractLogicalExp
 import org.apache.hyracks.algebricks.core.algebra.expressions.AggregateFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ConstantExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ExpressionRuntimeProvider;
+import org.apache.hyracks.algebricks.core.algebra.expressions.IAlgebricksConstantValue;
 import org.apache.hyracks.algebricks.core.algebra.expressions.IVariableTypeEnvironment;
 import org.apache.hyracks.algebricks.core.algebra.expressions.ScalarFunctionCallExpression;
 import org.apache.hyracks.algebricks.core.algebra.expressions.StatefulFunctionCallExpression;
@@ -204,7 +205,21 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule {
         public Pair<Boolean, ILogicalExpression> visitScalarFunctionCallExpression(ScalarFunctionCallExpression expr,
                 Void arg) throws AlgebricksException {
             boolean changed = constantFoldArgs(expr, arg);
-            if (!allArgsConstant(expr) || !expr.isFunctional() || !canConstantFold(expr)) {
+            List<Mutable<ILogicalExpression>> argList = expr.getArguments();
+            int argConstantCount = countConstantArgs(argList);
+            if (argConstantCount != argList.size()) {
+                if (argConstantCount > 0 && expr.getFunctionIdentifier().equals(BuiltinFunctions.OR)
+                        && expr.isFunctional()) {
+                    if (foldOrArgs(expr)) {
+                        ILogicalExpression changedExpr =
+                                expr.getArguments().size() == 1 ? expr.getArguments().get(0).getValue() : expr;
+                        return new Pair<>(true, changedExpr);
+                    }
+                }
+                return new Pair<>(changed, expr);
+            }
+
+            if (!expr.isFunctional() || !canConstantFold(expr)) {
                 return new Pair<>(changed, expr);
             }
 
@@ -348,13 +363,14 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule {
             return false;
         }
 
-        private boolean allArgsConstant(AbstractFunctionCallExpression expr) {
-            for (Mutable<ILogicalExpression> r : expr.getArguments()) {
-                if (r.getValue().getExpressionTag() != LogicalExpressionTag.CONSTANT) {
-                    return false;
+        private int countConstantArgs(List<Mutable<ILogicalExpression>> argList) {
+            int n = 0;
+            for (Mutable<ILogicalExpression> r : argList) {
+                if (r.getValue().getExpressionTag() == LogicalExpressionTag.CONSTANT) {
+                    n++;
                 }
             }
-            return true;
+            return n;
         }
 
         private boolean canConstantFold(ScalarFunctionCallExpression function) throws AlgebricksException {
@@ -391,5 +407,37 @@ public class ConstantFoldingRule implements IAlgebraicRewriteRule {
             }
             return true;
         }
+
+        private boolean foldOrArgs(ScalarFunctionCallExpression expr) {
+            // or(true,x,y) -> true; or(false,x,y) -> or(x,y)
+            boolean changed = false;
+            List<Mutable<ILogicalExpression>> argList = expr.getArguments();
+            Iterator<Mutable<ILogicalExpression>> argIter = argList.iterator();
+            Mutable<ILogicalExpression> argFalse = null;
+            while (argIter.hasNext()) {
+                Mutable<ILogicalExpression> argExprRef = argIter.next();
+                ILogicalExpression argExpr = argExprRef.getValue();
+                if (argExpr.getExpressionTag() != LogicalExpressionTag.CONSTANT) {
+                    continue;
+                }
+                ConstantExpression cExpr = (ConstantExpression) argExpr;
+                IAlgebricksConstantValue cValue = cExpr.getValue();
+                if (cValue.isTrue()) {
+                    // or(true,x,y) -> true;
+                    argList.clear();
+                    argList.add(argExprRef);
+                    return true;
+                } else if (cValue.isFalse()) {
+                    // remove 'false' from arg list, but save the expression.
+                    argFalse = argExprRef;
+                    argIter.remove();
+                    changed = true;
+                }
+            }
+            if (argList.isEmpty() && argFalse != null) {
+                argList.add(argFalse);
+            }
+            return changed;
+        }
     }
 }
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-01.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-01.sqlpp
new file mode 100644
index 0000000..bde8e75
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-01.sqlpp
@@ -0,0 +1,54 @@
+/*
+ * 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.
+ */
+/*
+ * Description: Test that BTree index is used in query plan
+ */
+
+// requesttype=application/json
+
+// param $p_fname:json="Julio"
+// param $p_lname:json=null
+
+drop  dataverse test if exists;
+create  dataverse test;
+
+use test;
+
+create type test.Emp as
+ closed {
+  id : bigint,
+  fname : string,
+  lname : string,
+  age : bigint,
+  dept : string
+};
+
+create  dataset employee(Emp) primary key id;
+
+create  index idx_employee_fname on employee (fname) type btree;
+
+create  index idx_employee_lname on employee (lname) type btree;
+
+select l.id, l.fname, l.lname, l.age
+from employee as l
+where
+  ($p_fname is null or l.fname = $p_fname )
+  and
+  ($p_lname is null or l.lname = $p_lname)
+order by l.id;
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-02.sqlpp b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-02.sqlpp
new file mode 100644
index 0000000..e74acc5
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/statement-params/statement-params-index-02.sqlpp
@@ -0,0 +1,54 @@
+/*
+ * 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.
+ */
+/*
+ * Description: Test that BTree index is used in query plan
+ */
+
+// requesttype=application/json
+
+// param $p_fname:json="Julio"
+// param $p_lname:json="Isa"
+
+drop  dataverse test if exists;
+create  dataverse test;
+
+use test;
+
+create type test.Emp as
+ closed {
+  id : bigint,
+  fname : string,
+  lname : string,
+  age : bigint,
+  dept : string
+};
+
+create  dataset employee(Emp) primary key id;
+
+create  index idx_employee_fname on employee (fname) type btree;
+
+create  index idx_employee_lname on employee (lname) type btree;
+
+select l.id, l.fname, l.lname, l.age
+from employee as l
+where
+  ($p_fname is null or l.fname = $p_fname )
+  and
+  ($p_lname is null or l.lname = $p_lname)
+order by l.id;
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-01.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-01.plan
new file mode 100644
index 0000000..0f02160
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-01.plan
@@ -0,0 +1,19 @@
+-- DISTRIBUTE_RESULT  |PARTITIONED|
+  -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+    -- STREAM_PROJECT  |PARTITIONED|
+      -- ASSIGN  |PARTITIONED|
+        -- SORT_MERGE_EXCHANGE [$$28(ASC) ]  |PARTITIONED|
+          -- STREAM_SELECT  |PARTITIONED|
+            -- STREAM_PROJECT  |PARTITIONED|
+              -- ASSIGN  |PARTITIONED|
+                -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                  -- BTREE_SEARCH  |PARTITIONED|
+                    -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                      -- STABLE_SORT [$$36(ASC)]  |PARTITIONED|
+                        -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                          -- STREAM_PROJECT  |PARTITIONED|
+                            -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                              -- BTREE_SEARCH  |PARTITIONED|
+                                -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                  -- ASSIGN  |PARTITIONED|
+                                    -- EMPTY_TUPLE_SOURCE  |PARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-02.plan b/asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-02.plan
new file mode 100644
index 0000000..1a1db2c
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/optimizerts/results/statement-params/statement-params-index-02.plan
@@ -0,0 +1,30 @@
+-- DISTRIBUTE_RESULT  |PARTITIONED|
+  -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+    -- STREAM_PROJECT  |PARTITIONED|
+      -- ASSIGN  |PARTITIONED|
+        -- SORT_MERGE_EXCHANGE [$$29(ASC) ]  |PARTITIONED|
+          -- STREAM_SELECT  |PARTITIONED|
+            -- STREAM_PROJECT  |PARTITIONED|
+              -- ASSIGN  |PARTITIONED|
+                -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                  -- BTREE_SEARCH  |PARTITIONED|
+                    -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                      -- INTERSECT  |PARTITIONED|
+                        -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                          -- STABLE_SORT [$$36(ASC)]  |PARTITIONED|
+                            -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                              -- STREAM_PROJECT  |PARTITIONED|
+                                -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                  -- BTREE_SEARCH  |PARTITIONED|
+                                    -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                      -- ASSIGN  |PARTITIONED|
+                                        -- EMPTY_TUPLE_SOURCE  |PARTITIONED|
+                        -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                          -- STABLE_SORT [$$40(ASC)]  |PARTITIONED|
+                            -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                              -- STREAM_PROJECT  |PARTITIONED|
+                                -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                  -- BTREE_SEARCH  |PARTITIONED|
+                                    -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                      -- ASSIGN  |PARTITIONED|
+                                        -- EMPTY_TUPLE_SOURCE  |PARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.2.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.2.query.sqlpp
new file mode 100644
index 0000000..83c3779
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.2.query.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: testing constant folding of OR with one TRUE argument
+ */
+
+select value true or x<=2
+from range(1, 4) x
+order by x;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.3.query.sqlpp
new file mode 100644
index 0000000..85a77fe
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.3.query.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: testing constant folding of OR with one TRUE argument
+ */
+
+explain select value true or x<=2
+from range(1, 4) x
+order by x;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.4.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.4.query.sqlpp
new file mode 100644
index 0000000..efb543f
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.4.query.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: testing constant folding of OR with one FALSE argument
+ */
+
+select value false or x<=2
+from range(1, 4) x
+order by x;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.5.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.5.query.sqlpp
new file mode 100644
index 0000000..4283fa6
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.5.query.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: testing constant folding of OR with one FALSE argument
+ */
+
+explain select value false or x<=2
+from range(1, 4) x
+order by x;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.6.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.6.query.sqlpp
new file mode 100644
index 0000000..b4b7c36
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.6.query.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: No constant folding of OR with non TRUE/FALSE argument
+ */
+
+explain select value null or x<=2
+from range(1, 4) x
+order by x;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.7.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.7.query.sqlpp
new file mode 100644
index 0000000..517a996
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/constant_folding/constant_folding.7.query.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/*
+ * Description: No constant folding of OR with a non functional argument
+ */
+
+explain select value true or get_year(current_date()) < x
+from range(1, 4) x
+order by x;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.1.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.1.ddl.sqlpp
new file mode 100644
index 0000000..7515b0b
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.1.ddl.sqlpp
@@ -0,0 +1,41 @@
+/*
+ * 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.
+ */
+/*
+ * Description: Test that BTree index is used in query plan
+ */
+
+drop  dataverse test if exists;
+create  dataverse test;
+
+use test;
+
+create type test.Emp as
+ closed {
+  id : bigint,
+  fname : string,
+  lname : string,
+  age : bigint,
+  dept : string
+};
+
+create  dataset employee(Emp) primary key id;
+
+create  index idx_employee_fname on employee (fname) type btree;
+
+create  index idx_employee_lname on employee (lname) type btree;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.2.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.2.update.sqlpp
new file mode 100644
index 0000000..eef17c1
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.2.update.sqlpp
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+/*
+ * Description: Test that BTree index is used in query plan
+ */
+
+use test;
+
+load  dataset employee using localfs ((`path`=`asterix_nc1://data/names.adm`),(`format`=`delimited-text`),(`delimiter`=`|`));
+
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.3.query.sqlpp
new file mode 100644
index 0000000..aae6b95
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.3.query.sqlpp
@@ -0,0 +1,36 @@
+/*
+ * 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.
+ */
+/*
+ * Description: Test that BTree index is used in this query plan
+ */
+
+// requesttype=application/json
+
+// param $p_fname:json="Julio"
+// param $p_lname:json=null
+
+use test;
+
+select l.id, l.fname, l.lname, l.age
+from employee as l
+where
+  ($p_fname is null or l.fname = $p_fname )
+  and
+  ($p_lname is null or l.lname = $p_lname)
+order by l.id;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.4.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.4.query.sqlpp
new file mode 100644
index 0000000..2b1c77c
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/statement-params/index_01/index_01.4.query.sqlpp
@@ -0,0 +1,36 @@
+/*
+ * 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.
+ */
+/*
+ * Description: Test that BTree index is used in this query plan
+ */
+
+// requesttype=application/json
+
+// param $p_fname:json="Julio"
+// param $p_lname:json="Isa"
+
+use test;
+
+select l.id, l.fname, l.lname, l.age
+from employee as l
+where
+  ($p_fname is null or l.fname = $p_fname )
+  and
+  ($p_lname is null or l.lname = $p_lname)
+order by l.id;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.2.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.2.adm
new file mode 100644
index 0000000..4b1e4bc
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.2.adm
@@ -0,0 +1,4 @@
+true
+true
+true
+true
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.3.adm
new file mode 100644
index 0000000..5b24cb2
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.3.adm
@@ -0,0 +1,18 @@
+distribute result [$$13]
+-- DISTRIBUTE_RESULT  |UNPARTITIONED|
+  exchange
+  -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+    assign [$$13] <- [TRUE]
+    -- ASSIGN  |UNPARTITIONED|
+      project ([])
+      -- STREAM_PROJECT  |UNPARTITIONED|
+        exchange
+        -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+          order (ASC, $$x)
+          -- STABLE_SORT [$$x(ASC)]  |UNPARTITIONED|
+            exchange
+            -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+              unnest $$x <- range(1, 4)
+              -- UNNEST  |UNPARTITIONED|
+                empty-tuple-source
+                -- EMPTY_TUPLE_SOURCE  |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.4.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.4.adm
new file mode 100644
index 0000000..5d73c0d
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.4.adm
@@ -0,0 +1,4 @@
+true
+true
+false
+false
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.5.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.5.adm
new file mode 100644
index 0000000..d3f67c2
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.5.adm
@@ -0,0 +1,18 @@
+distribute result [$$13]
+-- DISTRIBUTE_RESULT  |UNPARTITIONED|
+  exchange
+  -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+    project ([$$13])
+    -- STREAM_PROJECT  |UNPARTITIONED|
+      assign [$$13] <- [le($$x, 2)]
+      -- ASSIGN  |UNPARTITIONED|
+        exchange
+        -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+          order (ASC, $$x)
+          -- STABLE_SORT [$$x(ASC)]  |UNPARTITIONED|
+            exchange
+            -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+              unnest $$x <- range(1, 4)
+              -- UNNEST  |UNPARTITIONED|
+                empty-tuple-source
+                -- EMPTY_TUPLE_SOURCE  |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.6.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.6.adm
new file mode 100644
index 0000000..77dc263
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.6.adm
@@ -0,0 +1,18 @@
+distribute result [$$13]
+-- DISTRIBUTE_RESULT  |UNPARTITIONED|
+  exchange
+  -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+    project ([$$13])
+    -- STREAM_PROJECT  |UNPARTITIONED|
+      assign [$$13] <- [or(null, le($$x, 2))]
+      -- ASSIGN  |UNPARTITIONED|
+        exchange
+        -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+          order (ASC, $$x)
+          -- STABLE_SORT [$$x(ASC)]  |UNPARTITIONED|
+            exchange
+            -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+              unnest $$x <- range(1, 4)
+              -- UNNEST  |UNPARTITIONED|
+                empty-tuple-source
+                -- EMPTY_TUPLE_SOURCE  |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.7.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.7.adm
new file mode 100644
index 0000000..0fe9557
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/constant_folding/constant_folding.7.adm
@@ -0,0 +1,18 @@
+distribute result [$$15]
+-- DISTRIBUTE_RESULT  |UNPARTITIONED|
+  exchange
+  -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+    project ([$$15])
+    -- STREAM_PROJECT  |UNPARTITIONED|
+      assign [$$15] <- [or(TRUE, lt(get-year(current-date()), $$x))]
+      -- ASSIGN  |UNPARTITIONED|
+        exchange
+        -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+          order (ASC, $$x)
+          -- STABLE_SORT [$$x(ASC)]  |UNPARTITIONED|
+            exchange
+            -- ONE_TO_ONE_EXCHANGE  |UNPARTITIONED|
+              unnest $$x <- range(1, 4)
+              -- UNNEST  |UNPARTITIONED|
+                empty-tuple-source
+                -- EMPTY_TUPLE_SOURCE  |UNPARTITIONED|
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.3.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.3.adm
new file mode 100644
index 0000000..0881670
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.3.adm
@@ -0,0 +1,8 @@
+{ "id": 299, "fname": "Julio", "lname": "Iorio", "age": 37 }
+{ "id": 514, "fname": "Julio", "lname": "Ruben", "age": 41 }
+{ "id": 671, "fname": "Julio", "lname": "Vanpatten", "age": 30 }
+{ "id": 741, "fname": "Julio", "lname": "Mattocks", "age": 38 }
+{ "id": 881, "fname": "Julio", "lname": "Isa", "age": 38 }
+{ "id": 1418, "fname": "Julio", "lname": "Damore", "age": 27 }
+{ "id": 1430, "fname": "Julio", "lname": "Barkett", "age": 39 }
+{ "id": 8338, "fname": "Julio", "lname": "Bosket", "age": 28 }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.4.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.4.adm
new file mode 100644
index 0000000..87c7773
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/statement-params/index_01/index_01.4.adm
@@ -0,0 +1 @@
+{ "id": 881, "fname": "Julio", "lname": "Isa", "age": 38 }
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 63773dd..51ac565 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -8725,6 +8725,11 @@
   </test-group>
   <test-group name="statement-params">
     <test-case FilePath="statement-params">
+      <compilation-unit name="index_01">
+        <output-dir compare="Text">index_01</output-dir>
+      </compilation-unit>
+    </test-case>
+    <test-case FilePath="statement-params">
       <compilation-unit name="mixed_01">
         <output-dir compare="Text">mixed_01</output-dir>
       </compilation-unit>


[asterixdb] 04/06: [ASTERIXDB-2756][REP] Skip Replication of Dropped Indexes

Posted by mb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit 406b304d8f837062bd88dc7e27e4304a6cafb3ce
Author: Murtadha Hubail <mh...@apache.org>
AuthorDate: Wed Aug 19 19:13:51 2020 +0300

    [ASTERIXDB-2756][REP] Skip Replication of Dropped Indexes
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    
    - When attempting to asynchronously replicate an LSM
      component whose index was dropped, skip replication.
    
    Change-Id: Ifdbf61cdfb6099af43ba2d9353dde9e8e31a4701
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/7623
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Murtadha Hubail <mh...@apache.org>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
    Reviewed-by: Luo Chen <cl...@uci.edu>
---
 .../asterix/common/storage/DatasetResourceReference.java      |  2 ++
 .../replication/management/IndexReplicationManager.java       | 11 ++++++++---
 .../resource/PersistentLocalResourceRepository.java           |  5 +++--
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/DatasetResourceReference.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/DatasetResourceReference.java
index dc37fc2..6fd1c6a 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/DatasetResourceReference.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/DatasetResourceReference.java
@@ -19,6 +19,7 @@
 package org.apache.asterix.common.storage;
 
 import java.nio.file.Paths;
+import java.util.Objects;
 
 import org.apache.asterix.common.dataflow.DatasetLocalResource;
 import org.apache.asterix.common.utils.StorageConstants;
@@ -35,6 +36,7 @@ public class DatasetResourceReference extends ResourceReference {
     }
 
     public static DatasetResourceReference of(LocalResource localResource) {
+        Objects.requireNonNull(localResource);
         return parse(localResource);
     }
 
diff --git a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
index 4d660d2..dd953c4 100644
--- a/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
+++ b/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/IndexReplicationManager.java
@@ -131,9 +131,14 @@ public class IndexReplicationManager {
 
     private boolean skip(IReplicationJob job) {
         try {
-            final DatasetResourceReference indexFileRef =
-                    resourceRepository.getLocalResourceReference(job.getAnyFile());
-            return !replicationStrategy.isMatch(indexFileRef.getDatasetId());
+            final String fileToReplicate = job.getAnyFile();
+            final Optional<DatasetResourceReference> indexFileRefOpt =
+                    resourceRepository.getLocalResourceReference(fileToReplicate);
+            if (!indexFileRefOpt.isPresent()) {
+                LOGGER.warn("skipping replication of {} due to missing dataset resource reference", fileToReplicate);
+                return true;
+            }
+            return !replicationStrategy.isMatch(indexFileRefOpt.get().getDatasetId());
         } catch (HyracksDataException e) {
             throw new IllegalStateException("Couldn't find resource for " + job.getAnyFile(), e);
         }
diff --git a/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java b/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
index 145be86..508eb76 100644
--- a/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
+++ b/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
@@ -346,10 +346,11 @@ public class PersistentLocalResourceRepository implements ILocalResourceReposito
                 .collect(Collectors.toSet());
     }
 
-    public DatasetResourceReference getLocalResourceReference(String absoluteFilePath) throws HyracksDataException {
+    public Optional<DatasetResourceReference> getLocalResourceReference(String absoluteFilePath)
+            throws HyracksDataException {
         final String localResourcePath = StoragePathUtil.getIndexFileRelativePath(absoluteFilePath);
         final LocalResource lr = get(localResourcePath);
-        return DatasetResourceReference.of(lr);
+        return lr != null ? Optional.of(DatasetResourceReference.of(lr)) : Optional.empty();
     }
 
     /**


[asterixdb] 05/06: [NO ISSUE][RT] Fix upserting into secondary indexes

Posted by mb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit 0e432a003a308203db19ea752c64a1bd457d851f
Author: Ali Alsuliman <al...@gmail.com>
AuthorDate: Tue Aug 25 21:35:22 2020 -0700

    [NO ISSUE][RT] Fix upserting into secondary indexes
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    - do not upsert tuples that have a null/missing value
      in any SK field into secondary indexes (whether
      composite or non-composite ones).
    
    Change-Id: I9cc94de34b5ef1dfd5e1e7b6b9b35cc7316759ab
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/7443
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Michael Blow <mb...@apache.org>
---
 .../asterix/common/storage/IndexCheckpoint.java    |  2 +-
 .../LSMSecondaryUpsertOperatorNodePushable.java    | 41 +++++++++++++---------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java
index 3d0b9cb..878c94e 100644
--- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java
+++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/storage/IndexCheckpoint.java
@@ -38,7 +38,7 @@ public class IndexCheckpoint {
     private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
     private static final long INITIAL_CHECKPOINT_ID = 0;
     // TODO(mblow): remove this marker & related logic once we no longer are able to read indexes prior to the fix
-    private static final long HAS_NULL_MISSING_VALUES_FIX = -1;
+    private static final long HAS_NULL_MISSING_VALUES_FIX = -2;
     private long id;
     private long validComponentSequence;
     private long lowWatermark;
diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
index aa5775e..35ae904 100644
--- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
+++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/LSMSecondaryUpsertOperatorNodePushable.java
@@ -45,21 +45,20 @@ import org.apache.hyracks.storage.am.lsm.common.dataflow.LSMIndexInsertUpdateDel
  * This operator node is used for secondary indexes with upsert operations.
  * It works in the following way:
  * For each incoming tuple
- * -If old secondary keys == new secondary keys
+ * -If old secondary index tuple == new secondary index tuple
  * --do nothing
  * -else
- * --If old secondary keys are null?
+ * --If any old field is null/missing?
  * ---do nothing
  * --else
- * ---delete old secondary keys
- * --If new keys are null?
+ * ---delete old secondary index tuple
+ * --If any new field is null/missing?
  * ---do nothing
  * --else
- * ---insert new keys
+ * ---insert new secondary index tuple
  */
 public class LSMSecondaryUpsertOperatorNodePushable extends LSMIndexInsertUpdateDeleteOperatorNodePushable {
 
-    private static final int NULL_MISSING_FIELD_INDEX = 0;
     private final PermutingFrameTupleReference prevValueTuple = new PermutingFrameTupleReference();
     private final int upsertIndicatorFieldIndex;
     private final IBinaryBooleanInspector upsertIndicatorInspector;
@@ -105,9 +104,9 @@ public class LSMSecondaryUpsertOperatorNodePushable extends LSMIndexInsertUpdate
                 tuple.reset(accessor, i);
                 prevValueTuple.reset(accessor, i);
 
-                boolean isNewValueNullOrMissing = isNullOrMissing(tuple);
-                boolean isOldValueNullOrMissing = isNullOrMissing(prevValueTuple);
-                if (isNewValueNullOrMissing && isOldValueNullOrMissing) {
+                boolean newTupleHasNullOrMissing = hasNullOrMissing(tuple);
+                boolean oldTupleHasNullOrMissing = hasNullOrMissing(prevValueTuple);
+                if (newTupleHasNullOrMissing && oldTupleHasNullOrMissing) {
                     // No op
                     continue;
                 }
@@ -118,13 +117,13 @@ public class LSMSecondaryUpsertOperatorNodePushable extends LSMIndexInsertUpdate
                     // which are always the same
                     continue;
                 }
-                if (!isOldValueNullOrMissing) {
-                    // We need to delete previous
+                // if all old fields are known values, then delete. skip deleting if any is null or missing
+                if (!oldTupleHasNullOrMissing) {
                     abstractModCallback.setOp(Operation.DELETE);
                     lsmAccessor.forceDelete(prevValueTuple);
                 }
-                if (isUpsert && !isNewValueNullOrMissing) {
-                    // we need to insert the new value
+                // if all new fields are known values, then insert. skip inserting if any is null or missing
+                if (isUpsert && !newTupleHasNullOrMissing) {
                     abstractModCallback.setOp(Operation.INSERT);
                     lsmAccessor.forceInsert(tuple);
                 }
@@ -138,8 +137,18 @@ public class LSMSecondaryUpsertOperatorNodePushable extends LSMIndexInsertUpdate
         FrameUtils.flushFrame(writeBuffer.getBuffer(), writer);
     }
 
-    private static boolean isNullOrMissing(PermutingFrameTupleReference tuple) {
-        return TypeTagUtil.isType(tuple, NULL_MISSING_FIELD_INDEX, ATypeTag.SERIALIZED_NULL_TYPE_TAG)
-                || TypeTagUtil.isType(tuple, NULL_MISSING_FIELD_INDEX, ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
+    private boolean hasNullOrMissing(PermutingFrameTupleReference tuple) {
+        int fieldCount = tuple.getFieldCount();
+        for (int i = 0; i < fieldCount; i++) {
+            if (isNullOrMissing(tuple, i)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static boolean isNullOrMissing(PermutingFrameTupleReference tuple, int fieldIdx) {
+        return TypeTagUtil.isType(tuple, fieldIdx, ATypeTag.SERIALIZED_NULL_TYPE_TAG)
+                || TypeTagUtil.isType(tuple, fieldIdx, ATypeTag.SERIALIZED_MISSING_TYPE_TAG);
     }
 }


[asterixdb] 06/06: Merge branch 'gerrit/mad-hatter'

Posted by mb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit 798123df1ddcb06b89b356a2a6360544e6c9fa4f
Merge: dcb4ad2 0e432a0
Author: Michael Blow <mb...@apache.org>
AuthorDate: Fri Aug 28 08:35:10 2020 -0400

    Merge branch 'gerrit/mad-hatter'
    
    Change-Id: I2a86ca44953abfd2a7ff2cc5d5c9f340a68b356f

 .../optimizer/rules/ConstantFoldingRule.java       | 60 +++++++++++++++++++---
 .../rules/DisjunctivePredicateToJoinRule.java      | 24 +++++++--
 .../IntroduceSecondaryIndexInsertDeleteRule.java   | 37 ++++++++-----
 .../optimizer/rules/am/BTreeAccessMethod.java      |  3 ++
 .../translator/LangExpressionToPlanTranslator.java | 45 ++++++++--------
 .../SqlppExpressionToPlanTranslator.java           | 26 ++++++----
 .../skip-index/skip-secondary-btree-index-3.sqlpp  | 43 ++++++++++++++++
 .../statement-params-index-01.sqlpp                | 54 +++++++++++++++++++
 .../statement-params-index-02.sqlpp                | 54 +++++++++++++++++++
 .../skip-index/skip-secondary-btree-index-3.plan   | 15 ++++++
 .../statement-params-index-01.plan                 | 19 +++++++
 .../statement-params-index-02.plan                 | 30 +++++++++++
 .../constant_folding.2.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.3.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.4.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.5.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.6.query.sqlpp                 | 26 ++++++++++
 .../constant_folding.7.query.sqlpp                 | 26 ++++++++++
 .../statement-params/index_01/index_01.1.ddl.sqlpp | 41 +++++++++++++++
 .../index_01/index_01.2.update.sqlpp               | 26 ++++++++++
 .../index_01/index_01.3.query.sqlpp                | 36 +++++++++++++
 .../index_01/index_01.4.query.sqlpp                | 36 +++++++++++++
 .../misc/constant_folding/constant_folding.2.adm   |  4 ++
 .../misc/constant_folding/constant_folding.3.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.4.adm   |  4 ++
 .../misc/constant_folding/constant_folding.5.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.6.adm   | 18 +++++++
 .../misc/constant_folding/constant_folding.7.adm   | 18 +++++++
 .../statement-params/index_01/index_01.3.adm       |  8 +++
 .../statement-params/index_01/index_01.4.adm       |  1 +
 .../test/resources/runtimets/testsuite_sqlpp.xml   |  5 ++
 .../common/storage/DatasetResourceReference.java   |  2 +
 .../asterix/common/storage/IndexCheckpoint.java    |  2 +-
 .../visitor/OperatorExpressionVisitor.java         |  1 +
 .../management/IndexReplicationManager.java        | 11 ++--
 .../LSMSecondaryUpsertOperatorNodePushable.java    | 41 +++++++++------
 .../PersistentLocalResourceRepository.java         |  5 +-
 .../algebra/util/OperatorManipulationUtil.java     | 12 +++++
 .../AbstractIntroduceGroupByCombinerRule.java      |  2 +-
 .../rules/EnforceStructuralPropertiesRule.java     |  5 +-
 .../rules/ExtractCommonExpressionsRule.java        |  1 +
 .../rewriter/rules/ExtractCommonOperatorsRule.java | 17 +++---
 .../rewriter/rules/IntroduceProjectsRule.java      |  1 +
 .../rules/subplan/PushSubplanIntoGroupByRule.java  | 13 +++--
 44 files changed, 821 insertions(+), 91 deletions(-)

diff --cc asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
index 8c35ea3,47e0373..014ee40
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
@@@ -402,21 -408,36 +418,53 @@@ public class ConstantFoldingRule implem
              return true;
          }
  
+         private boolean foldOrArgs(ScalarFunctionCallExpression expr) {
+             // or(true,x,y) -> true; or(false,x,y) -> or(x,y)
+             boolean changed = false;
+             List<Mutable<ILogicalExpression>> argList = expr.getArguments();
+             Iterator<Mutable<ILogicalExpression>> argIter = argList.iterator();
+             Mutable<ILogicalExpression> argFalse = null;
+             while (argIter.hasNext()) {
+                 Mutable<ILogicalExpression> argExprRef = argIter.next();
+                 ILogicalExpression argExpr = argExprRef.getValue();
+                 if (argExpr.getExpressionTag() != LogicalExpressionTag.CONSTANT) {
+                     continue;
+                 }
+                 ConstantExpression cExpr = (ConstantExpression) argExpr;
+                 IAlgebricksConstantValue cValue = cExpr.getValue();
+                 if (cValue.isTrue()) {
+                     // or(true,x,y) -> true;
+                     argList.clear();
+                     argList.add(argExprRef);
+                     return true;
+                 } else if (cValue.isFalse()) {
+                     // remove 'false' from arg list, but save the expression.
+                     argFalse = argExprRef;
+                     argIter.remove();
+                     changed = true;
+                 }
+             }
+             if (argList.isEmpty() && argFalse != null) {
+                 argList.add(argFalse);
+             }
+             return changed;
+         }
++
 +        // IEvaluatorContext
 +
 +        @Override
 +        public IServiceContext getServiceContext() {
 +            return serviceContext;
 +        }
 +
 +        @Override
 +        public IHyracksTaskContext getTaskContext() {
 +            return null;
 +        }
 +
 +        @Override
 +        public IWarningCollector getWarningCollector() {
 +            return warningCollector;
 +        }
      }
  }
diff --cc asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java
index 43136c0,fe03d75..e1bbacf
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/DisjunctivePredicateToJoinRule.java
@@@ -142,8 -154,9 +154,9 @@@ public class DisjunctivePredicateToJoin
  
          ILogicalExpression cExp = new ConstantExpression(new AsterixConstantValue(list));
          Mutable<ILogicalExpression> mutCExp = new MutableObject<>(cExp);
-         IFunctionInfo scanFctInfo = BuiltinFunctions.getBuiltinFunctionInfo(BuiltinFunctions.SCAN_COLLECTION);
-         UnnestingFunctionCallExpression scanExp = new UnnestingFunctionCallExpression(scanFctInfo, mutCExp);
+         UnnestingFunctionCallExpression scanExp = new UnnestingFunctionCallExpression(
 -                BuiltinFunctions.getAsterixFunctionInfo(BuiltinFunctions.SCAN_COLLECTION));
++                BuiltinFunctions.getBuiltinFunctionInfo(BuiltinFunctions.SCAN_COLLECTION));
+         scanExp.getArguments().add(mutCExp);
          scanExp.setSourceLocation(sourceLoc);
          LogicalVariable scanVar = context.newVar();
          UnnestOperator unn = new UnnestOperator(scanVar, new MutableObject<>(scanExp));
diff --cc asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
index 17bc77c,e8ceba4..d38a263
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
@@@ -442,7 -440,7 +439,7 @@@ abstract class LangExpressionToPlanTran
          return plan;
      }
  
-     protected ILogicalOperator translateDelete(DatasetDataSource targetDatasource, Mutable<ILogicalExpression> varRef,
 -    private ILogicalOperator translateDelete(DatasetDataSource targetDatasource, LogicalVariable resVar,
++    protected ILogicalOperator translateDelete(DatasetDataSource targetDatasource, LogicalVariable resVar,
              List<Mutable<ILogicalExpression>> varRefsForLoading,
              List<Mutable<ILogicalExpression>> additionalFilteringExpressions, ILogicalOperator assign,
              ICompiledDmlStatement stmt) throws AlgebricksException {
@@@ -463,7 -463,7 +462,7 @@@
          return leafOperator;
      }
  
-     protected ILogicalOperator translateUpsert(DatasetDataSource targetDatasource, Mutable<ILogicalExpression> varRef,
 -    private ILogicalOperator translateUpsert(DatasetDataSource targetDatasource, LogicalVariable resVar,
++    protected ILogicalOperator translateUpsert(DatasetDataSource targetDatasource, LogicalVariable resVar,
              List<Mutable<ILogicalExpression>> varRefsForLoading,
              List<Mutable<ILogicalExpression>> additionalFilteringExpressions, ILogicalOperator assign,
              List<String> additionalFilteringField, LogicalVariable unnestVar, ILogicalOperator topOp,
@@@ -578,7 -582,7 +581,7 @@@
          return processReturningExpression(rootOperator, upsertOp, compiledUpsert, resultMetadata);
      }
  
-     protected ILogicalOperator translateInsert(DatasetDataSource targetDatasource, Mutable<ILogicalExpression> varRef,
 -    private ILogicalOperator translateInsert(DatasetDataSource targetDatasource, LogicalVariable resVar,
++    protected ILogicalOperator translateInsert(DatasetDataSource targetDatasource, LogicalVariable resVar,
              List<Mutable<ILogicalExpression>> varRefsForLoading,
              List<Mutable<ILogicalExpression>> additionalFilteringExpressions, ILogicalOperator assign,
              ICompiledDmlStatement stmt, IResultMetadata resultMetadata) throws AlgebricksException {


[asterixdb] 02/06: [NO ISSUE][COMP] Fix reference sharing in some optimizer rules

Posted by mb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mblow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit e08456c537465dedde44e62e8f7b8a2be2a13769
Author: Dmitry Lychagin <dm...@couchbase.com>
AuthorDate: Mon Aug 17 11:00:01 2020 -0700

    [NO ISSUE][COMP] Fix reference sharing in some optimizer rules
    
    Details:
    - Fixed optimizer rules that reused same operator/expression
      references or instances when creating new operators
    - Fixed optimizer rules that reported that they did not make
      any plan changes when, in fact, they did
    
    Change-Id: Ib9846f47339ea6e06fda17f4bac08a99ca5e8406
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/7406
    Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
---
 .../IntroduceSecondaryIndexInsertDeleteRule.java   | 37 ++++++++++++------
 .../optimizer/rules/am/BTreeAccessMethod.java      |  3 ++
 .../translator/LangExpressionToPlanTranslator.java | 45 ++++++++++++----------
 .../algebra/util/OperatorManipulationUtil.java     | 12 ++++++
 .../AbstractIntroduceGroupByCombinerRule.java      |  2 +-
 .../rules/EnforceStructuralPropertiesRule.java     |  5 +--
 .../rules/ExtractCommonExpressionsRule.java        |  1 +
 .../rewriter/rules/ExtractCommonOperatorsRule.java | 17 +++-----
 .../rewriter/rules/IntroduceProjectsRule.java      |  1 +
 .../rules/subplan/PushSubplanIntoGroupByRule.java  | 13 +++++--
 10 files changed, 86 insertions(+), 50 deletions(-)

diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java
index 6c258e4..37b6ca7 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java
@@ -75,6 +75,7 @@ import org.apache.hyracks.algebricks.core.algebra.operators.logical.InsertDelete
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.InsertDeleteUpsertOperator.Kind;
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.ReplicateOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.TokenizeOperator;
+import org.apache.hyracks.algebricks.core.algebra.util.OperatorManipulationUtil;
 import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
 import org.apache.hyracks.api.exceptions.SourceLocation;
 
@@ -350,31 +351,40 @@ public class IntroduceSecondaryIndexInsertDeleteRule implements IAlgebraicRewrit
 
                     // TokenizeOperator to tokenize [SK, PK] pairs
                     TokenizeOperator tokenUpdate = new TokenizeOperator(dataSourceIndex,
-                            primaryIndexModificationOp.getPrimaryKeyExpressions(), secondaryExpressions,
-                            tokenizeKeyVars, filterExpression, primaryIndexModificationOp.getOperation(),
-                            primaryIndexModificationOp.isBulkload(), isPartitioned, varTypes);
+                            OperatorManipulationUtil
+                                    .cloneExpressions(primaryIndexModificationOp.getPrimaryKeyExpressions()),
+                            secondaryExpressions, tokenizeKeyVars,
+                            filterExpression != null
+                                    ? new MutableObject<>(filterExpression.getValue().cloneExpression()) : null,
+                            primaryIndexModificationOp.getOperation(), primaryIndexModificationOp.isBulkload(),
+                            isPartitioned, varTypes);
                     tokenUpdate.setSourceLocation(sourceLoc);
                     tokenUpdate.getInputs().add(new MutableObject<ILogicalOperator>(currentTop));
                     context.computeAndSetTypeEnvironmentForOperator(tokenUpdate);
                     replicateOutput = tokenUpdate;
                     indexUpdate = new IndexInsertDeleteUpsertOperator(dataSourceIndex,
-                            primaryIndexModificationOp.getPrimaryKeyExpressions(), tokenizeKeyExprs, filterExpression,
-                            primaryIndexModificationOp.getOperation(), primaryIndexModificationOp.isBulkload(),
+                            OperatorManipulationUtil
+                                    .cloneExpressions(primaryIndexModificationOp.getPrimaryKeyExpressions()),
+                            tokenizeKeyExprs, filterExpression, primaryIndexModificationOp.getOperation(),
+                            primaryIndexModificationOp.isBulkload(),
                             primaryIndexModificationOp.getAdditionalNonFilteringExpressions() == null ? 0
                                     : primaryIndexModificationOp.getAdditionalNonFilteringExpressions().size());
                     indexUpdate.setSourceLocation(sourceLoc);
-                    indexUpdate.setAdditionalFilteringExpressions(filteringExpressions);
+                    indexUpdate.setAdditionalFilteringExpressions(
+                            OperatorManipulationUtil.cloneExpressions(filteringExpressions));
                     indexUpdate.getInputs().add(new MutableObject<ILogicalOperator>(tokenUpdate));
                 } else {
                     // When TokenizeOperator is not needed
                     indexUpdate = new IndexInsertDeleteUpsertOperator(dataSourceIndex,
-                            primaryIndexModificationOp.getPrimaryKeyExpressions(), secondaryExpressions,
-                            filterExpression, primaryIndexModificationOp.getOperation(),
+                            OperatorManipulationUtil
+                                    .cloneExpressions(primaryIndexModificationOp.getPrimaryKeyExpressions()),
+                            secondaryExpressions, filterExpression, primaryIndexModificationOp.getOperation(),
                             primaryIndexModificationOp.isBulkload(),
                             primaryIndexModificationOp.getAdditionalNonFilteringExpressions() == null ? 0
                                     : primaryIndexModificationOp.getAdditionalNonFilteringExpressions().size());
                     indexUpdate.setSourceLocation(sourceLoc);
-                    indexUpdate.setAdditionalFilteringExpressions(filteringExpressions);
+                    indexUpdate.setAdditionalFilteringExpressions(
+                            OperatorManipulationUtil.cloneExpressions(filteringExpressions));
                     replicateOutput = indexUpdate;
                     // We add the necessary expressions for upsert
                     if (primaryIndexModificationOp.getOperation() == Kind.UPSERT) {
@@ -478,12 +488,15 @@ public class IntroduceSecondaryIndexInsertDeleteRule implements IAlgebraicRewrit
                 }
                 DataSourceIndex dataSourceIndex = new DataSourceIndex(index, dataverseName, datasetName, mp);
                 indexUpdate = new IndexInsertDeleteUpsertOperator(dataSourceIndex,
-                        primaryIndexModificationOp.getPrimaryKeyExpressions(), secondaryExpressions, filterExpression,
-                        primaryIndexModificationOp.getOperation(), primaryIndexModificationOp.isBulkload(),
+                        OperatorManipulationUtil
+                                .cloneExpressions(primaryIndexModificationOp.getPrimaryKeyExpressions()),
+                        secondaryExpressions, filterExpression, primaryIndexModificationOp.getOperation(),
+                        primaryIndexModificationOp.isBulkload(),
                         primaryIndexModificationOp.getAdditionalNonFilteringExpressions() == null ? 0
                                 : primaryIndexModificationOp.getAdditionalNonFilteringExpressions().size());
                 indexUpdate.setSourceLocation(sourceLoc);
-                indexUpdate.setAdditionalFilteringExpressions(filteringExpressions);
+                indexUpdate.setAdditionalFilteringExpressions(
+                        OperatorManipulationUtil.cloneExpressions(filteringExpressions));
                 if (primaryIndexModificationOp.getOperation() == Kind.UPSERT) {
                     // set before op secondary key expressions
                     if (filteringFields != null) {
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
index 164a505..87a2d03 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/BTreeAccessMethod.java
@@ -792,6 +792,9 @@ public class BTreeAccessMethod implements IAccessMethod {
             } else {
                 keyVar = ((VariableReferenceExpression) searchKeyExpr).getVariableReference();
                 if (constExpression != null) {
+                    if (constExpression.getExpressionTag() != LogicalExpressionTag.CONSTANT) {
+                        constExpression = constExpression.cloneExpression();
+                    }
                     assignKeyExprList.add(new MutableObject<>(constExpression));
                     assignKeyVarList.add(constExprVars[i]);
                 }
diff --git a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
index 5851467..e8ceba4 100644
--- a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
+++ b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java
@@ -413,22 +413,19 @@ class LangExpressionToPlanTranslator
                 assign.getInputs().add(new MutableObject<>(topOp));
             }
 
-            VariableReferenceExpression resVarRef2 = new VariableReferenceExpression(resVar);
-            resVarRef2.setSourceLocation(sourceLoc);
-            Mutable<ILogicalExpression> varRef = new MutableObject<>(resVarRef2);
             ILogicalOperator leafOperator;
             switch (stmt.getKind()) {
                 case INSERT:
-                    leafOperator = translateInsert(targetDatasource, varRef, varRefsForLoading,
+                    leafOperator = translateInsert(targetDatasource, resVar, varRefsForLoading,
                             additionalFilteringExpressions, assign, stmt, resultMetadata);
                     break;
                 case UPSERT:
-                    leafOperator = translateUpsert(targetDatasource, varRef, varRefsForLoading,
+                    leafOperator = translateUpsert(targetDatasource, resVar, varRefsForLoading,
                             additionalFilteringExpressions, assign, additionalFilteringField, unnestVar, topOp, exprs,
-                            resVar, additionalFilteringAssign, stmt, resultMetadata);
+                            additionalFilteringAssign, stmt, resultMetadata);
                     break;
                 case DELETE:
-                    leafOperator = translateDelete(targetDatasource, varRef, varRefsForLoading,
+                    leafOperator = translateDelete(targetDatasource, resVar, varRefsForLoading,
                             additionalFilteringExpressions, assign, stmt);
                     break;
                 default:
@@ -443,7 +440,7 @@ class LangExpressionToPlanTranslator
         return plan;
     }
 
-    private ILogicalOperator translateDelete(DatasetDataSource targetDatasource, Mutable<ILogicalExpression> varRef,
+    private ILogicalOperator translateDelete(DatasetDataSource targetDatasource, LogicalVariable resVar,
             List<Mutable<ILogicalExpression>> varRefsForLoading,
             List<Mutable<ILogicalExpression>> additionalFilteringExpressions, ILogicalOperator assign,
             ICompiledDmlStatement stmt) throws AlgebricksException {
@@ -453,8 +450,10 @@ class LangExpressionToPlanTranslator
                     targetDatasource.getDataset().getDatasetName()
                             + ": delete from dataset is not supported on Datasets with Meta records");
         }
-        InsertDeleteUpsertOperator deleteOp = new InsertDeleteUpsertOperator(targetDatasource, varRef,
-                varRefsForLoading, InsertDeleteUpsertOperator.Kind.DELETE, false);
+        VariableReferenceExpression varRef = new VariableReferenceExpression(resVar);
+        varRef.setSourceLocation(stmt.getSourceLocation());
+        InsertDeleteUpsertOperator deleteOp = new InsertDeleteUpsertOperator(targetDatasource,
+                new MutableObject<>(varRef), varRefsForLoading, InsertDeleteUpsertOperator.Kind.DELETE, false);
         deleteOp.setAdditionalFilteringExpressions(additionalFilteringExpressions);
         deleteOp.getInputs().add(new MutableObject<>(assign));
         deleteOp.setSourceLocation(sourceLoc);
@@ -464,11 +463,11 @@ class LangExpressionToPlanTranslator
         return leafOperator;
     }
 
-    private ILogicalOperator translateUpsert(DatasetDataSource targetDatasource, Mutable<ILogicalExpression> varRef,
+    private ILogicalOperator translateUpsert(DatasetDataSource targetDatasource, LogicalVariable resVar,
             List<Mutable<ILogicalExpression>> varRefsForLoading,
             List<Mutable<ILogicalExpression>> additionalFilteringExpressions, ILogicalOperator assign,
             List<String> additionalFilteringField, LogicalVariable unnestVar, ILogicalOperator topOp,
-            List<Mutable<ILogicalExpression>> exprs, LogicalVariable resVar, AssignOperator additionalFilteringAssign,
+            List<Mutable<ILogicalExpression>> exprs, AssignOperator additionalFilteringAssign,
             ICompiledDmlStatement stmt, IResultMetadata resultMetadata) throws AlgebricksException {
         SourceLocation sourceLoc = stmt.getSourceLocation();
         if (!targetDatasource.getDataset().allow(topOp, DatasetUtil.OP_UPSERT)) {
@@ -521,8 +520,10 @@ class LangExpressionToPlanTranslator
                 }
             }
             // A change feed, we don't need the assign to access PKs
-            upsertOp = new InsertDeleteUpsertOperator(targetDatasource, varRef, varRefsForLoading, metaExpSingletonList,
-                    InsertDeleteUpsertOperator.Kind.UPSERT, false);
+            VariableReferenceExpression varRef = new VariableReferenceExpression(resVar);
+            varRef.setSourceLocation(stmt.getSourceLocation());
+            upsertOp = new InsertDeleteUpsertOperator(targetDatasource, new MutableObject<>(varRef), varRefsForLoading,
+                    metaExpSingletonList, InsertDeleteUpsertOperator.Kind.UPSERT, false);
             upsertOp.setUpsertIndicatorVar(context.newVar());
             upsertOp.setUpsertIndicatorVarType(BuiltinType.ABOOLEAN);
             // Create and add a new variable used for representing the original record
@@ -554,7 +555,9 @@ class LangExpressionToPlanTranslator
             topOp.getInputs().set(0, new MutableObject<>(metaAndKeysAssign));
             upsertOp.setAdditionalFilteringExpressions(additionalFilteringExpressions);
         } else {
-            upsertOp = new InsertDeleteUpsertOperator(targetDatasource, varRef, varRefsForLoading,
+            VariableReferenceExpression varRef = new VariableReferenceExpression(resVar);
+            varRef.setSourceLocation(stmt.getSourceLocation());
+            upsertOp = new InsertDeleteUpsertOperator(targetDatasource, new MutableObject<>(varRef), varRefsForLoading,
                     InsertDeleteUpsertOperator.Kind.UPSERT, false);
             upsertOp.setAdditionalFilteringExpressions(additionalFilteringExpressions);
             upsertOp.getInputs().add(new MutableObject<>(assign));
@@ -579,7 +582,7 @@ class LangExpressionToPlanTranslator
         return processReturningExpression(rootOperator, upsertOp, compiledUpsert, resultMetadata);
     }
 
-    private ILogicalOperator translateInsert(DatasetDataSource targetDatasource, Mutable<ILogicalExpression> varRef,
+    private ILogicalOperator translateInsert(DatasetDataSource targetDatasource, LogicalVariable resVar,
             List<Mutable<ILogicalExpression>> varRefsForLoading,
             List<Mutable<ILogicalExpression>> additionalFilteringExpressions, ILogicalOperator assign,
             ICompiledDmlStatement stmt, IResultMetadata resultMetadata) throws AlgebricksException {
@@ -590,8 +593,10 @@ class LangExpressionToPlanTranslator
                             + ": insert into dataset is not supported on Datasets with Meta records");
         }
         // Adds the insert operator.
-        InsertDeleteUpsertOperator insertOp = new InsertDeleteUpsertOperator(targetDatasource, varRef,
-                varRefsForLoading, InsertDeleteUpsertOperator.Kind.INSERT, false);
+        VariableReferenceExpression varRef = new VariableReferenceExpression(resVar);
+        varRef.setSourceLocation(stmt.getSourceLocation());
+        InsertDeleteUpsertOperator insertOp = new InsertDeleteUpsertOperator(targetDatasource,
+                new MutableObject<>(varRef), varRefsForLoading, InsertDeleteUpsertOperator.Kind.INSERT, false);
         insertOp.setAdditionalFilteringExpressions(additionalFilteringExpressions);
         insertOp.getInputs().add(new MutableObject<>(assign));
         insertOp.setSourceLocation(sourceLoc);
@@ -620,8 +625,8 @@ class LangExpressionToPlanTranslator
 
         //Create an assign operator that makes the variable used by the return expression
         LogicalVariable insertedVar = context.newVar();
-        AssignOperator insertedVarAssignOperator =
-                new AssignOperator(insertedVar, new MutableObject<>(insertOp.getPayloadExpression().getValue()));
+        AssignOperator insertedVarAssignOperator = new AssignOperator(insertedVar,
+                new MutableObject<>(insertOp.getPayloadExpression().getValue().cloneExpression()));
         insertedVarAssignOperator.getInputs().add(insertOp.getInputs().get(0));
         insertedVarAssignOperator.setSourceLocation(sourceLoc);
         insertOp.getInputs().set(0, new MutableObject<>(insertedVarAssignOperator));
diff --git a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java
index 3f5012a..b8fe5c7 100644
--- a/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java
+++ b/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/util/OperatorManipulationUtil.java
@@ -53,6 +53,7 @@ import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.Ope
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.visitors.VariableUtilities;
 import org.apache.hyracks.algebricks.core.algebra.plan.ALogicalPlanImpl;
 import org.apache.hyracks.algebricks.core.algebra.typing.ITypingContext;
+import org.apache.hyracks.api.exceptions.SourceLocation;
 
 public class OperatorManipulationUtil {
 
@@ -461,4 +462,15 @@ public class OperatorManipulationUtil {
         }
         return -1;
     }
+
+    public static List<Mutable<ILogicalExpression>> createVariableReferences(List<LogicalVariable> varList,
+            SourceLocation sourceLoc) {
+        List<Mutable<ILogicalExpression>> varRefs = new ArrayList<>(varList.size());
+        for (LogicalVariable var : varList) {
+            VariableReferenceExpression varRef = new VariableReferenceExpression(var);
+            varRef.setSourceLocation(sourceLoc);
+            varRefs.add(new MutableObject<>(varRef));
+        }
+        return varRefs;
+    }
 }
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java
index 6c42929..b917ce1 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java
@@ -87,7 +87,7 @@ public abstract class AbstractIntroduceGroupByCombinerRule extends AbstractIntro
             if (!newGbyLiveVars.contains(usedVar)) {
                 // Let the left-hand side of gbyOp's decoration expressions populated through the combiner group-by without
                 // any intermediate assignment.
-                newGbyOp.addDecorExpression(null, p.second.getValue());
+                newGbyOp.addDecorExpression(null, p.second.getValue().cloneExpression());
                 newGbyLiveVars.add(usedVar);
             }
         }
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/EnforceStructuralPropertiesRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/EnforceStructuralPropertiesRule.java
index 706028b..9af21f5 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/EnforceStructuralPropertiesRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/EnforceStructuralPropertiesRule.java
@@ -698,9 +698,8 @@ public class EnforceStructuralPropertiesRule implements IAlgebraicRewriteRule {
         // these two exchange ops are needed so that the parents of replicate stay the same during later optimizations.
         // This is because replicate operator has references to its parents. If any later optimizations add new parents,
         // then replicate would still point to the old ones.
-        MutableObject<ILogicalOperator> replicateOpRef = new MutableObject<>(replicateOp);
-        ExchangeOperator exchToLocalAgg = createOneToOneExchangeOp(replicateOpRef, ctx);
-        ExchangeOperator exchToForward = createOneToOneExchangeOp(replicateOpRef, ctx);
+        ExchangeOperator exchToLocalAgg = createOneToOneExchangeOp(new MutableObject<>(replicateOp), ctx);
+        ExchangeOperator exchToForward = createOneToOneExchangeOp(new MutableObject<>(replicateOp), ctx);
         MutableObject<ILogicalOperator> exchToLocalAggRef = new MutableObject<>(exchToLocalAgg);
         MutableObject<ILogicalOperator> exchToForwardRef = new MutableObject<>(exchToForward);
 
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java
index b8dd24f..2cfa241 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonExpressionsRule.java
@@ -256,6 +256,7 @@ public class ExtractCommonExpressionsRule implements IAlgebraicRewriteRule {
                     }
                 } else {
                     if (expr.isFunctional() && assignCommonExpression(exprEqClass, expr)) {
+                        modified = true;
                         //re-obtain the live vars after rewriting in the method called in the if condition
                         Set<LogicalVariable> liveVars = new HashSet<LogicalVariable>();
                         VariableUtilities.getLiveVariables(op, liveVars);
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
index 176ab7a..3effcc8 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractCommonOperatorsRule.java
@@ -35,7 +35,6 @@ import org.apache.hyracks.algebricks.core.algebra.base.ILogicalOperator;
 import org.apache.hyracks.algebricks.core.algebra.base.IOptimizationContext;
 import org.apache.hyracks.algebricks.core.algebra.base.LogicalOperatorTag;
 import org.apache.hyracks.algebricks.core.algebra.base.LogicalVariable;
-import org.apache.hyracks.algebricks.core.algebra.expressions.VariableReferenceExpression;
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractLogicalOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.AbstractReplicateOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.logical.AssignOperator;
@@ -48,6 +47,7 @@ import org.apache.hyracks.algebricks.core.algebra.operators.physical.AssignPOper
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.OneToOneExchangePOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.ReplicatePOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.StreamProjectPOperator;
+import org.apache.hyracks.algebricks.core.algebra.util.OperatorManipulationUtil;
 import org.apache.hyracks.algebricks.core.rewriter.base.IAlgebraicRewriteRule;
 import org.apache.hyracks.api.exceptions.SourceLocation;
 
@@ -172,7 +172,6 @@ public class ExtractCommonOperatorsRule implements IAlgebraicRewriteRule {
             ReplicateOperator rop = new ReplicateOperator(group.size(), materializationFlags);
             rop.setSourceLocation(candidateSourceLoc);
             rop.setPhysicalOperator(new ReplicatePOperator());
-            Mutable<ILogicalOperator> ropRef = new MutableObject<ILogicalOperator>(rop);
             AbstractLogicalOperator aopCandidate = (AbstractLogicalOperator) candidate.getValue();
             List<Mutable<ILogicalOperator>> originalCandidateParents = childrenToParents.get(candidate);
 
@@ -194,14 +193,14 @@ public class ExtractCommonOperatorsRule implements IAlgebraicRewriteRule {
                 AbstractLogicalOperator parent = (AbstractLogicalOperator) parentRef.getValue();
                 int index = parent.getInputs().indexOf(candidate);
                 if (parent.getOperatorTag() == LogicalOperatorTag.EXCHANGE) {
-                    parent.getInputs().set(index, ropRef);
+                    parent.getInputs().set(index, new MutableObject<>(rop));
                     rop.getOutputs().add(parentRef);
                 } else {
                     AbstractLogicalOperator exchange = new ExchangeOperator();
                     exchange.setPhysicalOperator(new OneToOneExchangePOperator());
                     exchange.setExecutionMode(rop.getExecutionMode());
                     MutableObject<ILogicalOperator> exchangeRef = new MutableObject<ILogicalOperator>(exchange);
-                    exchange.getInputs().add(ropRef);
+                    exchange.getInputs().add(new MutableObject<>(rop));
                     rop.getOutputs().add(exchangeRef);
                     context.computeAndSetTypeEnvironmentForOperator(exchange);
                     parent.getInputs().set(index, exchangeRef);
@@ -210,12 +209,6 @@ public class ExtractCommonOperatorsRule implements IAlgebraicRewriteRule {
             }
             List<LogicalVariable> liveVarsNew = new ArrayList<LogicalVariable>();
             VariableUtilities.getLiveVariables(candidate.getValue(), liveVarsNew);
-            ArrayList<Mutable<ILogicalExpression>> assignExprs = new ArrayList<Mutable<ILogicalExpression>>();
-            for (LogicalVariable liveVar : liveVarsNew) {
-                VariableReferenceExpression liveVarRef = new VariableReferenceExpression(liveVar);
-                liveVarRef.setSourceLocation(candidateSourceLoc);
-                assignExprs.add(new MutableObject<ILogicalExpression>(liveVarRef));
-            }
             for (Mutable<ILogicalOperator> ref : group) {
                 if (ref.equals(candidate)) {
                     continue;
@@ -230,6 +223,8 @@ public class ExtractCommonOperatorsRule implements IAlgebraicRewriteRule {
 
                 SourceLocation refSourceLoc = ref.getValue().getSourceLocation();
 
+                List<Mutable<ILogicalExpression>> assignExprs =
+                        OperatorManipulationUtil.createVariableReferences(liveVarsNew, candidateSourceLoc);
                 AbstractLogicalOperator assignOperator = new AssignOperator(liveVars, assignExprs);
                 assignOperator.setSourceLocation(refSourceLoc);
                 assignOperator.setExecutionMode(rop.getExecutionMode());
@@ -241,7 +236,7 @@ public class ExtractCommonOperatorsRule implements IAlgebraicRewriteRule {
                 AbstractLogicalOperator exchOp = new ExchangeOperator();
                 exchOp.setPhysicalOperator(new OneToOneExchangePOperator());
                 exchOp.setExecutionMode(rop.getExecutionMode());
-                exchOp.getInputs().add(ropRef);
+                exchOp.getInputs().add(new MutableObject<>(rop));
                 MutableObject<ILogicalOperator> exchOpRef = new MutableObject<ILogicalOperator>(exchOp);
                 rop.getOutputs().add(exchOpRef);
                 assignOperator.getInputs().add(exchOpRef);
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/IntroduceProjectsRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/IntroduceProjectsRule.java
index a724014..af67be2 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/IntroduceProjectsRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/IntroduceProjectsRule.java
@@ -177,6 +177,7 @@ public class IntroduceProjectsRule implements IAlgebraicRewriteRule {
             if (liveVars.size() == projectVarsTemp.size() && liveVars.containsAll(projectVarsTemp)) {
                 // The existing project has become useless. Remove it.
                 parentOp.getInputs().get(parentInputIndex).setValue(op.getInputs().get(0).getValue());
+                modified = true;
             }
         }
 
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/PushSubplanIntoGroupByRule.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/PushSubplanIntoGroupByRule.java
index 8e41a15..6d5d67d 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/PushSubplanIntoGroupByRule.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/subplan/PushSubplanIntoGroupByRule.java
@@ -204,7 +204,7 @@ public class PushSubplanIntoGroupByRule implements IAlgebraicRewriteRule {
                             newGbyNestedPlans.add(new ALogicalPlanImpl(rootOpRef));
 
                             upperSubplanRootRefIterator.remove();
-                            changed |= true;
+                            changed = true;
                             break;
                         }
                     }
@@ -212,10 +212,12 @@ public class PushSubplanIntoGroupByRule implements IAlgebraicRewriteRule {
 
                 if (upperSubplanRootRefs.isEmpty()) {
                     subplanNestedPlanIterator.remove();
+                    changed = true;
                 }
             }
             if (subplan.getNestedPlans().isEmpty()) {
                 subplanOperatorIterator.remove();
+                changed = true;
             }
         }
 
@@ -228,7 +230,7 @@ public class PushSubplanIntoGroupByRule implements IAlgebraicRewriteRule {
         parent.getInputs().get(0).setValue(gby);
 
         // Removes unnecessary pipelines inside the group by operator.
-        cleanup(currentRootRef.getValue(), gby);
+        changed |= cleanup(currentRootRef.getValue(), gby);
 
         // Computes type environments.
         context.computeAndSetTypeEnvironmentForOperator(gby);
@@ -245,7 +247,8 @@ public class PushSubplanIntoGroupByRule implements IAlgebraicRewriteRule {
      *            the group-by operator.
      * @throws AlgebricksException
      */
-    private void cleanup(ILogicalOperator rootOp, GroupByOperator gby) throws AlgebricksException {
+    private boolean cleanup(ILogicalOperator rootOp, GroupByOperator gby) throws AlgebricksException {
+        boolean changed = false;
         Set<LogicalVariable> freeVars = new HashSet<>();
         OperatorPropertiesUtil.getFreeVariablesInPath(rootOp, gby, freeVars);
         Iterator<ILogicalPlan> nestedPlanIterator = gby.getNestedPlans().iterator();
@@ -259,16 +262,20 @@ public class PushSubplanIntoGroupByRule implements IAlgebraicRewriteRule {
                     if (!freeVars.contains(aggOp.getVariables().get(varIndex))) {
                         aggOp.getVariables().remove(varIndex);
                         aggOp.getExpressions().remove(varIndex);
+                        changed = true;
                     }
                 }
                 if (aggOp.getVariables().isEmpty()) {
                     nestRootRefIterator.remove();
+                    changed = true;
                 }
             }
             if (nestedPlan.getRoots().isEmpty()) {
                 nestedPlanIterator.remove();
+                changed = true;
             }
         }
+        return changed;
     }
 
     private Mutable<ILogicalOperator> downToNts(Mutable<ILogicalOperator> opRef) {