You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by dl...@apache.org on 2019/07/31 17:41:21 UTC

[asterixdb] branch master updated: [NO ISSUE][COMP] Emit warning for cross product

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 7689fa8  [NO ISSUE][COMP] Emit warning for cross product
7689fa8 is described below

commit 7689fa8f6a054494cbdc2a90bc92c678b5852a75
Author: Dmitry Lychagin <dm...@couchbase.com>
AuthorDate: Tue Jul 30 13:49:45 2019 -0700

    [NO ISSUE][COMP] Emit warning for cross product
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    - Emit warning for a cross product join
    
    Change-Id: Icd2b27f23517842fc532db7741ee60e8da737082
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/3512
    Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
    Reviewed-by: Dmitry Lychagin <dm...@couchbase.com>
    Reviewed-by: Ali Alsuliman <al...@gmail.com>
---
 .../plan-warning/plan-warning.1.query.sqlpp        | 32 ++++++++++++++++++++++
 .../warnings/plan-warning/plan-warning.1.adm       |  8 ++++++
 .../test/resources/runtimets/testsuite_sqlpp.xml   |  6 ++++
 .../lang/sqlpp/visitor/DeepCopyVisitor.java        |  1 +
 .../algebricks/rewriter/util/JoinUtils.java        | 17 ++++++++++--
 .../apache/hyracks/api/exceptions/ErrorCode.java   |  1 +
 .../src/main/resources/errormsg/en.properties      |  1 +
 7 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/plan-warning/plan-warning.1.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/plan-warning/plan-warning.1.query.sqlpp
new file mode 100644
index 0000000..1e3d045
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/plan-warning/plan-warning.1.query.sqlpp
@@ -0,0 +1,32 @@
+/*
+ * 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  : Warning if cross product
+ * Expected     : SUCCESS (with HYR10007 warning)
+ */
+
+with ds as (
+  from range(1, 4) r
+  select r % 2 as x, r as y
+)
+
+from ds d1, ds d2
+select distinct d1.x, d2.y
+order by x, y
\ No newline at end of file
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/plan-warning/plan-warning.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/plan-warning/plan-warning.1.adm
new file mode 100644
index 0000000..cb184cc
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/plan-warning/plan-warning.1.adm
@@ -0,0 +1,8 @@
+{ "x": 0, "y": 1 }
+{ "x": 0, "y": 2 }
+{ "x": 0, "y": 3 }
+{ "x": 0, "y": 4 }
+{ "x": 1, "y": 1 }
+{ "x": 1, "y": 2 }
+{ "x": 1, "y": 3 }
+{ "x": 1, "y": 4 }
\ 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 511feab..aab320f 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -12363,6 +12363,12 @@
       </compilation-unit>
     </test-case>
     <test-case FilePath="warnings" check-warnings="true">
+      <compilation-unit name="plan-warning">
+        <output-dir compare="Text">plan-warning</output-dir>
+        <expected-warn>HYR10007: Encountered a cross product join</expected-warn>
+      </compilation-unit>
+    </test-case>
+    <test-case FilePath="warnings" check-warnings="true">
       <compilation-unit name="unknown-hint-warning">
         <output-dir compare="Text">unknown-hint-warning</output-dir>
         <expected-warn>ASX1107: Unknown hint: unknown_hint_groupby. Supported hints are: hash</expected-warn>
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
index 6e28d36..b1b39e6 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
+++ b/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/DeepCopyVisitor.java
@@ -361,6 +361,7 @@ public class DeepCopyVisitor extends AbstractSqlppQueryExpressionVisitor<ILangEx
         }
 
         SelectExpression copy = new SelectExpression(lets, select, orderby, limit, selectExpression.isSubquery());
+        copy.setSourceLocation(select.getSourceLocation());
         copy.addHints(selectExpression.getHints());
 
         return copy;
diff --git a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java
index 6687027..34ce4f2 100644
--- a/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java
+++ b/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/util/JoinUtils.java
@@ -45,7 +45,11 @@ import org.apache.hyracks.algebricks.core.algebra.operators.physical.HybridHashJ
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.InMemoryHashJoinPOperator;
 import org.apache.hyracks.algebricks.core.algebra.operators.physical.NestedLoopJoinPOperator;
 import org.apache.hyracks.algebricks.core.algebra.properties.ILogicalPropertiesVector;
+import org.apache.hyracks.algebricks.core.algebra.util.OperatorPropertiesUtil;
 import org.apache.hyracks.algebricks.core.config.AlgebricksConfig;
+import org.apache.hyracks.api.exceptions.ErrorCode;
+import org.apache.hyracks.api.exceptions.SourceLocation;
+import org.apache.hyracks.api.exceptions.Warning;
 
 public class JoinUtils {
     private JoinUtils() {
@@ -60,8 +64,9 @@ public class JoinUtils {
         List<LogicalVariable> sideRight = new LinkedList<>();
         List<LogicalVariable> varsLeft = op.getInputs().get(0).getValue().getSchema();
         List<LogicalVariable> varsRight = op.getInputs().get(1).getValue().getSchema();
-        if (isHashJoinCondition(op.getCondition().getValue(), varsLeft, varsRight, sideLeft, sideRight)) {
-            BroadcastSide side = getBroadcastJoinSide(op.getCondition().getValue(), varsLeft, varsRight);
+        ILogicalExpression conditionExpr = op.getCondition().getValue();
+        if (isHashJoinCondition(conditionExpr, varsLeft, varsRight, sideLeft, sideRight)) {
+            BroadcastSide side = getBroadcastJoinSide(conditionExpr, varsLeft, varsRight);
             if (side == null) {
                 setHashJoinOp(op, JoinPartitioningType.PAIRWISE, sideLeft, sideRight, context);
             } else {
@@ -87,6 +92,7 @@ public class JoinUtils {
                 }
             }
         } else {
+            warnIfCrossProduct(conditionExpr, op.getSourceLocation(), context);
             setNestedLoopJoinOp(op);
         }
     }
@@ -233,4 +239,11 @@ public class JoinUtils {
             }
         }
     }
+
+    private static void warnIfCrossProduct(ILogicalExpression conditionExpr, SourceLocation sourceLoc,
+            IOptimizationContext context) {
+        if (OperatorPropertiesUtil.isAlwaysTrueCond(conditionExpr) && sourceLoc != null) {
+            context.getWarningCollector().warn(Warning.forHyracks(sourceLoc, ErrorCode.CROSS_PRODUCT_JOIN));
+        }
+    }
 }
diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
index 4d6dc7d..dd16ae2 100644
--- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
+++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java
@@ -164,6 +164,7 @@ public class ErrorCode {
     public static final int EXPR_NOT_NORMALIZED = 10004;
     public static final int OPERATOR_NOT_IMPLEMENTED = 10005;
     public static final int INAPPLICABLE_HINT = 10006;
+    public static final int CROSS_PRODUCT_JOIN = 10007;
 
     private static class Holder {
         private static final Map<Integer, String> errorMessageMap;
diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties b/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
index f73f130..69ad48c 100644
--- a/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
+++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
@@ -146,3 +146,4 @@
 10004 = Expression has not been normalized
 10005 = Operator is not implemented: %1$s
 10006 = Could not apply %1$s hint: %2$s
+10007 = Encountered a cross product join