You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2024/09/13 19:24:32 UTC
Change in asterixdb[neo]: [ASTERIXDB-3502][STO] Incorrectly treating two expressions as commuta...
From Ritik Raj <ra...@gmail.com>:
Ritik Raj has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18875 )
Change subject: [ASTERIXDB-3502][STO] Incorrectly treating two expressions as commutative
......................................................................
[ASTERIXDB-3502][STO] Incorrectly treating two expressions as commutative
- user model changes: no
- storage format changes: no
- interface changes: no
- Incorrectly considering two different expression as commutative
Ext-ref: MB-63481
Change-Id: Ia83d1559f5f45406075a384ce25251e50b362b5b
---
A asterixdb/asterix-app/src/test/resources/runtimets/results/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.adm
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.02.update.sqlpp
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.01.ddl.sqlpp
M asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.query.sqlpp
M asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java
6 files changed, 149 insertions(+), 15 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/75/18875/1
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.01.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.01.ddl.sqlpp
new file mode 100644
index 0000000..e4c59d2
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.01.ddl.sqlpp
@@ -0,0 +1,61 @@
+/*
+ * 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 tpch IF EXISTS;
+CREATE DATAVERSE tpch;
+
+USE tpch;
+
+CREATE TYPE tpch.CustomerType AS
+ CLOSED {
+ c_custkey : integer,
+ c_name : string,
+ c_address : string,
+ c_nationkey : integer,
+ c_phone : string,
+ c_acctbal : double,
+ c_mktsegment : string,
+ c_comment : string
+};
+
+CREATE TYPE tpch.SupplierType AS
+ CLOSED {
+ s_suppkey : integer,
+ s_name : string,
+ s_address : string,
+ s_nationkey : integer,
+ s_phone : string,
+ s_acctbal : double,
+ s_comment : string
+};
+
+CREATE TYPE tpch.NationType AS
+ CLOSED {
+ n_nationkey : integer,
+ n_name : string,
+ n_regionkey : integer,
+ n_comment : string
+};
+
+CREATE DATASET Supplier(SupplierType) PRIMARY KEY s_suppkey;
+
+CREATE DATASET Nation(NationType) PRIMARY KEY n_nationkey;
+
+CREATE DATASET Customer(CustomerType) PRIMARY KEY c_custkey;
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.02.update.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.02.update.sqlpp
new file mode 100644
index 0000000..b4ae2b6
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.02.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.
+ */
+
+USE tpch;
+
+LOAD DATASET Supplier USING localfs ((`path`=`asterix_nc1://data/tpch0.001/supplier.tbl`),(`format`=`delimited-text`),(`delimiter`=`|`));
+
+LOAD DATASET Nation USING localfs ((`path`=`asterix_nc1://data/tpch0.001/nation.tbl`),(`format`=`delimited-text`),(`delimiter`=`|`));
+
+LOAD DATASET Customer USING localfs ((`path`=`asterix_nc1://data/tpch0.001/customer.tbl`),(`format`=`delimited-text`),(`delimiter`=`|`));
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.query.sqlpp
new file mode 100644
index 0000000..76750f9
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.query.sqlpp
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+/*
+ * Similar to hash-join-with-redundant-variable.04.query.sqlpp
+ * But with Index NL. The plan of this test should has three
+ * hash-partition-exchange (as opposed to test 13 & 14). Because the parallelism
+ * is set to 3, then the last join requires both sides to be hash partitioned.
+ * Customer will need to duplicate its variable to join both with Nation and Supplier.
+ * This is the effect of using Index NL with parallelism != # of partitions
+ */
+
+USE tpch;
+
+-- this query should not give any results
+
+SELECT n.n_nationkey, s.s_nationkey, c.c_nationkey
+FROM Nation n, Supplier s, Customer c
+WHERE s.s_nationkey = n.n_nationkey
+AND c.c_nationkey = n.n_nationkey
+AND (s.s_nationkey = c.c_nationkey) = (s.s_nationkey = c.c_nationkey)
+-- before ASTERIXDB-3502, below expression was getting removed, hence evaluated to wrong result
+AND (s.s_nationkey != c.c_nationkey) = (s.s_nationkey = c.c_nationkey)
+ORDER BY n.n_nationkey, s.s_nationkey, c.c_nationkey;
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.adm
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/join/ASTERIXDB-3502/hash-join-with-redundant-variable.03.adm
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml b/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
index c63de24..717700f 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
@@ -6655,6 +6655,11 @@
</compilation-unit>
</test-case>
<test-case FilePath="join">
+ <compilation-unit name="ASTERIXDB-3502">
+ <output-dir compare="Text">ASTERIXDB-3502</output-dir>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="join">
<compilation-unit name="hash_join_array">
<output-dir compare="Text">hash_join_array</output-dir>
</compilation-unit>
diff --git a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java
index cce0bd8..24b38ab 100644
--- a/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java
+++ b/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/util/FunctionUtil.java
@@ -347,30 +347,17 @@
return expr1.equals(expr2);
}
- return commutativeEquals(expr1, expr2, new BitSet());
- }
-
- private static boolean commutativeEquals(ILogicalExpression expr1, ILogicalExpression expr2, BitSet matched) {
- if (expr1.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL
- || expr2.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL) {
- return expr1.equals(expr2);
- }
-
- AbstractFunctionCallExpression funcExpr1 = (AbstractFunctionCallExpression) expr1;
- AbstractFunctionCallExpression funcExpr2 = (AbstractFunctionCallExpression) expr2;
-
List<Mutable<ILogicalExpression>> args1 = funcExpr1.getArguments();
List<Mutable<ILogicalExpression>> args2 = funcExpr2.getArguments();
- BitSet childrenSet = new BitSet();
+ BitSet matched = new BitSet();
int numberOfMatches = 0;
for (Mutable<ILogicalExpression> arg1 : args1) {
int prevNumberOfMatches = numberOfMatches;
for (int i = 0; i < args2.size(); i++) {
Mutable<ILogicalExpression> arg2 = args2.get(i);
- childrenSet.clear();
- if (!matched.get(i) && commutativeEquals(arg1.getValue(), arg2.getValue(), childrenSet)) {
+ if (!matched.get(i) && commutativeEquals(arg1.getValue(), arg2.getValue())) {
matched.set(i);
numberOfMatches++;
break;
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18875
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: Ia83d1559f5f45406075a384ce25251e50b362b5b
Gerrit-Change-Number: 18875
Gerrit-PatchSet: 1
Gerrit-Owner: Ritik Raj <ra...@gmail.com>
Gerrit-MessageType: newchange