>From Ali Alsuliman <[email protected]>:

Ali Alsuliman has submitted this change. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21174?usp=email )

Change subject: [ASTERIXDB-3766][COMP] Fix NPE with LOJ ON condition having NOT 
EXISTS
......................................................................

[ASTERIXDB-3766][COMP] Fix NPE with LOJ ON condition having NOT EXISTS

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

Details:
Substitute decor variables that get assigned to new variables
in the nested plan of the group-by and not just the root op
of the nested plan.

Ext-ref: MB-71638

Change-Id: I8212c42c9c4f83851a9274840a36700524413a4d
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21174
Reviewed-by: Ali Alsuliman <[email protected]>
Tested-by: Jenkins <[email protected]>
Reviewed-by: Preetham Poluparthi <[email protected]>
Integration-Tests: Jenkins <[email protected]>
---
A 
asterixdb/asterix-app/src/test/resources/optimizerts/queries/leftouterjoin/not_exists.sqlpp
A 
asterixdb/asterix-app/src/test/resources/optimizerts/results/leftouterjoin/not_exists.plan
M 
hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/AbstractIntroduceGroupByCombinerRule.java
3 files changed, 148 insertions(+), 2 deletions(-)

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

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




diff --git 
a/asterixdb/asterix-app/src/test/resources/optimizerts/queries/leftouterjoin/not_exists.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/leftouterjoin/not_exists.sqlpp
new file mode 100644
index 0000000..4347b9e
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/optimizerts/queries/leftouterjoin/not_exists.sqlpp
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+
+DROP DATAVERSE test IF EXISTS;
+CREATE DATAVERSE test;
+USE test;
+
+CREATE TYPE type1 AS {id: int};
+CREATE DATASET a1(type1) PRIMARY KEY id;
+CREATE DATASET a2(type1) PRIMARY KEY id;
+
+SELECT a1.c_id
+FROM a1 a1
+LEFT JOIN a2 a21 ON a1.c_id = a21.c_id AND NOT EXISTS
+(SELECT 1 FROM a2 a22 WHERE a21.c_id = a22.c_id AND a21.part = a22.part AND 
a22.tm > a21.tm);
\ No newline at end of file
diff --git 
a/asterixdb/asterix-app/src/test/resources/optimizerts/results/leftouterjoin/not_exists.plan
 
b/asterixdb/asterix-app/src/test/resources/optimizerts/results/leftouterjoin/not_exists.plan
new file mode 100644
index 0000000..c3c5742
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/optimizerts/results/leftouterjoin/not_exists.plan
@@ -0,0 +1,115 @@
+distribute result [$$67]
+-- DISTRIBUTE_RESULT  |PARTITIONED|
+  exchange
+  -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+    assign [$$67] <- [{"c_id": $$72}] project: [$$67]
+    -- ASSIGN  |PARTITIONED|
+      outer-unnest $$64 <- scan-collection($$63)
+      -- LEFT_OUTER_UNNEST  |PARTITIONED|
+        project ([$$63, $$72])
+        -- STREAM_PROJECT  |PARTITIONED|
+          exchange
+          -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+            group by ([$$84 := $$86]) decor ([$$72]) {
+                      aggregate [$$63] <- [listify($$89)]
+                      -- AGGREGATE  |LOCAL|
+                        select (not(neq($$71, 0)))
+                        -- STREAM_SELECT  |LOCAL|
+                          project ([$$71, $$89])
+                          -- STREAM_PROJECT  |LOCAL|
+                            group by ([$$82 := $$87]) decor ([$$86; $$a1; 
$$89; $$72; $$90; $$91; $$92]) {
+                                      aggregate [$$71] <- [agg-sum($$85)]
+                                      -- AGGREGATE  |LOCAL|
+                                        nested tuple source
+                                        -- NESTED_TUPLE_SOURCE  |LOCAL|
+                                   }
+                            -- MICRO_PRE_CLUSTERED_GROUP_BY[$$87]  |LOCAL|
+                              order (ASC, $$87)
+                              -- MICRO_STABLE_SORT [$$87(ASC)]  |LOCAL|
+                                select (not(is-missing($$88))) project: [$$85, 
$$86, $$87, $$a1, $$72, $$89, $$90, $$91, $$92]
+                                -- STREAM_SELECT  |LOCAL|
+                                  nested tuple source
+                                  -- NESTED_TUPLE_SOURCE  |LOCAL|
+                   }
+            -- PRE_CLUSTERED_GROUP_BY[$$86]  |PARTITIONED|
+              exchange
+              -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                order (ASC, $$86)
+                -- STABLE_SORT [$$86(ASC)]  |PARTITIONED|
+                  exchange
+                  -- HASH_PARTITION_EXCHANGE [$$86]  |PARTITIONED|
+                    group by ([$$86 := $$68; $$87 := $$69]) decor ([$$a1; 
$$72; $$89 := $$a21; $$90 := $$73; $$91 := $$74; $$92 := $$77; $$88 := $$83]) {
+                              aggregate [$$85] <- [agg-count({ "$1": 1 })]
+                              -- AGGREGATE  |LOCAL|
+                                select (not(is-missing($$81)))
+                                -- STREAM_SELECT  |LOCAL|
+                                  project ([$$81])
+                                  -- STREAM_PROJECT  |LOCAL|
+                                    nested tuple source
+                                    -- NESTED_TUPLE_SOURCE  |LOCAL|
+                           }
+                    -- PRE_CLUSTERED_GROUP_BY[$$68, $$69]  |PARTITIONED|
+                      exchange
+                      -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                        order (ASC, $$68) (ASC, $$69)
+                        -- STABLE_SORT [$$68(ASC), $$69(ASC)]  |PARTITIONED|
+                          exchange
+                          -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                            project ([$$72, $$a1, $$81, $$68, $$69, $$83, 
$$a21, $$73, $$74, $$77])
+                            -- STREAM_PROJECT  |PARTITIONED|
+                              exchange
+                              -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                left outer join (and(eq($$74, $$75), gt($$76, 
$$77), eq($$73, $$79)))
+                                -- HYBRID_HASH_JOIN [$$74, $$73][$$75, $$79]  
|PARTITIONED|
+                                  exchange
+                                  -- HASH_PARTITION_EXCHANGE [$$74, $$73]  
|PARTITIONED|
+                                    left outer join (eq($$72, $$73))
+                                    -- HYBRID_HASH_JOIN [$$72][$$73]  
|PARTITIONED|
+                                      exchange
+                                      -- HASH_PARTITION_EXCHANGE [$$72]  
|PARTITIONED|
+                                        assign [$$72] <- 
[$$a1.getField("c_id")]
+                                        -- ASSIGN  |PARTITIONED|
+                                          exchange
+                                          -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                            data-scan []<-[$$68, $$a1] <- 
test.a1
+                                            -- DATASOURCE_SCAN  |PARTITIONED|
+                                              exchange
+                                              -- ONE_TO_ONE_EXCHANGE  
|PARTITIONED|
+                                                empty-tuple-source
+                                                -- EMPTY_TUPLE_SOURCE  
|PARTITIONED|
+                                      exchange
+                                      -- HASH_PARTITION_EXCHANGE [$$73]  
|PARTITIONED|
+                                        assign [$$83, $$77, $$73, $$74] <- 
[true, $$a21.getField("tm"), $$a21.getField("c_id"), $$a21.getField("part")]
+                                        -- ASSIGN  |PARTITIONED|
+                                          assign [$$69, $$a21] <- [$$70, 
$$a22] project: [$$69, $$a21]
+                                          -- ASSIGN  |PARTITIONED|
+                                            exchange
+                                            -- ONE_TO_ONE_EXCHANGE  
|PARTITIONED|
+                                              replicate
+                                              -- REPLICATE  |PARTITIONED|
+                                                exchange
+                                                -- ONE_TO_ONE_EXCHANGE  
|PARTITIONED|
+                                                  data-scan []<-[$$70, $$a22] 
<- test.a2
+                                                  -- DATASOURCE_SCAN  
|PARTITIONED|
+                                                    exchange
+                                                    -- ONE_TO_ONE_EXCHANGE  
|PARTITIONED|
+                                                      empty-tuple-source
+                                                      -- EMPTY_TUPLE_SOURCE  
|PARTITIONED|
+                                  exchange
+                                  -- HASH_PARTITION_EXCHANGE [$$75, $$79]  
|PARTITIONED|
+                                    assign [$$81, $$79, $$76, $$75] <- [true, 
$$a22.getField("c_id"), $$a22.getField("tm"), $$a22.getField("part")] project: 
[$$81, $$75, $$76, $$79]
+                                    -- ASSIGN  |PARTITIONED|
+                                      project ([$$a22])
+                                      -- STREAM_PROJECT  |PARTITIONED|
+                                        exchange
+                                        -- ONE_TO_ONE_EXCHANGE  |PARTITIONED|
+                                          replicate
+                                          -- REPLICATE  |PARTITIONED|
+                                            exchange
+                                            -- ONE_TO_ONE_EXCHANGE  
|PARTITIONED|
+                                              data-scan []<-[$$70, $$a22] <- 
test.a2
+                                              -- DATASOURCE_SCAN  |PARTITIONED|
+                                                exchange
+                                                -- ONE_TO_ONE_EXCHANGE  
|PARTITIONED|
+                                                  empty-tuple-source
+                                                  -- EMPTY_TUPLE_SOURCE  
|PARTITIONED|
\ No newline at end of file
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 b917ce1..93b048b 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
@@ -107,8 +107,8 @@
                 VariableReferenceExpression varRef = new 
VariableReferenceExpression(var);
                 varRef.setSourceLocation(gbyOp.getSourceLocation());
                 newGbyOp.addDecorExpression(newDecorVar, varRef);
-                
VariableUtilities.substituteVariables(gbyOp.getNestedPlans().get(0).getRoots().get(0).getValue(),
 var,
-                        newDecorVar, context);
+                VariableUtilities.substituteVariablesInDescendantsAndSelf(
+                        
gbyOp.getNestedPlans().get(0).getRoots().get(0).getValue(), var, newDecorVar, 
context);
             }
         }


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

Gerrit-MessageType: merged
Gerrit-Project: asterixdb
Gerrit-Branch: lumina
Gerrit-Change-Id: I8212c42c9c4f83851a9274840a36700524413a4d
Gerrit-Change-Number: 21174
Gerrit-PatchSet: 3
Gerrit-Owner: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Ali Alsuliman <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Janhavi Tripurwar <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Preetham Poluparthi <[email protected]>
Gerrit-Reviewer: Shahrzad Shirazi <[email protected]>

Reply via email to