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

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

commit c70bae02ff9346b1fee7da101cd0cc67cf0733ce
Author: Vijay Sarathy <[email protected]>
AuthorDate: Mon Mar 13 13:24:05 2023 -0700

    [ASTERIXDB-3135] Cardinality hints enhanced error checking
    
    Change-Id: I2b260845d3122ae31d4dd461e23e131ac064521c
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17420
    Integration-Tests: Jenkins <[email protected]>
    Tested-by: Jenkins <[email protected]>
    Reviewed-by: Vijay Sarathy <[email protected]>
    Reviewed-by: Ali Alsuliman <[email protected]>
---
 .../apache/asterix/optimizer/rules/cbo/Stats.java  | 33 ++++++++++++++++++++--
 .../cardinality-hint-warning.05.query.sqlpp        |  2 +-
 .../cardinality-hint-warning.06.query.sqlpp        |  4 +--
 .../cardinality-hint-warning.07.query.sqlpp        |  2 +-
 .../cardinality-hint-warning.08.query.sqlpp        |  2 +-
 .../cardinality-hint-warning.09.query.sqlpp        |  6 ++--
 ...lpp => cardinality-hint-warning.10.query.sqlpp} |  0
 ...lpp => cardinality-hint-warning.11.query.sqlpp} |  0
 ...lpp => cardinality-hint-warning.12.query.sqlpp} |  0
 ...lpp => cardinality-hint-warning.13.query.sqlpp} |  2 +-
 ...lpp => cardinality-hint-warning.14.query.sqlpp} |  2 +-
 .../cardinality-hint-warning.10.adm                |  1 +
 .../cardinality-hint-warning.11.adm                |  1 +
 .../cardinality-hint-warning.12.adm                |  1 +
 .../cardinality-hint-warning.13.adm                |  1 +
 .../cardinality-hint-warning.14.adm                |  1 +
 .../test/resources/runtimets/testsuite_sqlpp.xml   |  6 +++-
 .../asterix-lang-sqlpp/src/main/javacc/SQLPP.jj    |  4 +--
 18 files changed, 52 insertions(+), 16 deletions(-)

diff --git 
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java
 
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java
index 96bc4124a4..b3c4876a4d 100644
--- 
a/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java
+++ 
b/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java
@@ -41,6 +41,9 @@ import 
org.apache.hyracks.algebricks.core.algebra.functions.AlgebricksBuiltinFun
 import 
org.apache.hyracks.algebricks.core.algebra.operators.logical.DataSourceScanOperator;
 import 
org.apache.hyracks.algebricks.core.algebra.operators.logical.SelectOperator;
 import 
org.apache.hyracks.algebricks.core.algebra.operators.logical.SubplanOperator;
+import org.apache.hyracks.api.exceptions.ErrorCode;
+import org.apache.hyracks.api.exceptions.IWarningCollector;
+import org.apache.hyracks.api.exceptions.Warning;
 
 public class Stats {
 
@@ -94,12 +97,27 @@ public class Stats {
             int leftIndex = 
joinEnum.findJoinNodeIndexByName(anno.getLeftSideDataSet());
             if (leftIndex != idx1 && leftIndex != idx2) {
                 // should not happen
+                IWarningCollector warningCollector = 
joinEnum.optCtx.getWarningCollector();
+                if (warningCollector.shouldWarn()) {
+                    
warningCollector.warn(Warning.of(joinExpr.getSourceLocation(), 
ErrorCode.INAPPLICABLE_HINT,
+                            "productivity", "Invalid collection name/alias: " 
+ anno.getLeftSideDataSet()));
+                }
+                return 1.0;
+            }
+            double productivity = anno.getJoinProductivity();
+            if (productivity <= 0) {
+                IWarningCollector warningCollector = 
joinEnum.optCtx.getWarningCollector();
+                if (warningCollector.shouldWarn()) {
+                    
warningCollector.warn(Warning.of(joinExpr.getSourceLocation(), 
ErrorCode.INAPPLICABLE_HINT,
+                            "productivity",
+                            "Productivity specified: " + productivity + ", has 
to be a decimal value greater than 0"));
+                }
                 return 1.0;
             }
             if (leftIndex == idx1) {
-                return anno.getJoinProductivity() / card2;
+                return productivity / card2;
             } else {
-                return anno.getJoinProductivity() / card1;
+                return productivity / card1;
             }
         } else {
             if (card1 < card2) {
@@ -152,7 +170,16 @@ public class Stats {
         PredicateCardinalityAnnotation pca = 
afcExpr.getAnnotation(PredicateCardinalityAnnotation.class);
         if (pca != null) {
             s = pca.getSelectivity();
-            sel *= s;
+            if (s <= 0 || s >= 1) {
+                IWarningCollector warningCollector = 
joinEnum.optCtx.getWarningCollector();
+                if (warningCollector.shouldWarn()) {
+                    
warningCollector.warn(Warning.of(afcExpr.getSourceLocation(), 
ErrorCode.INAPPLICABLE_HINT,
+                            "selectivity", "Selectivity specified: " + s
+                                    + ", has to be a decimal value greater 
than 0 and less than 1"));
+                }
+            } else {
+                sel *= s;
+            }
         } else {
             JoinProductivityAnnotation jpa = 
afcExpr.getAnnotation(JoinProductivityAnnotation.class);
             s = findJoinSelectivity(jpa, afcExpr);
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.05.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.05.query.sqlpp
index 0d98c86be6..ddce731f04 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.05.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.05.query.sqlpp
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity */ = n.n_nationkey;
+WHERE c.c_nationkey = n.n_nationkey and n.n_name /*+ selectivity -0.1 */ = 
'UNITED STATES';
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.06.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.06.query.sqlpp
index 21d4735f92..659988dca2 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.06.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.06.query.sqlpp
@@ -17,7 +17,7 @@
  * under the License.
  */
 /*
-* Description  : Test warnings for enhanced hash join hint
+* Description  : Test warnings for cardinality hints
 * Expected Res : Warning, ignore hint
 * Date         : 03/09/2023
 */
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity 100.0 */ = n.n_nationkey;
+WHERE c.c_nationkey = n.n_nationkey and n.n_name /*+ selectivity 0.0 */ = 
'UNITED STATES';
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.07.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.07.query.sqlpp
index 626490a3ef..8c196dfe91 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.07.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.07.query.sqlpp
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity n */ = n.n_nationkey;
+WHERE c.c_nationkey = n.n_nationkey and n.n_name /*+ selectivity 1.0 */ = 
'UNITED STATES';
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
index 6a6a440489..0d98c86be6 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity n 100 */ = n.n_nationkey;
+WHERE c.c_nationkey /*+ productivity */ = n.n_nationkey;
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.09.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.09.query.sqlpp
index 725b0eac35..21d4735f92 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.09.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.09.query.sqlpp
@@ -17,8 +17,8 @@
  * under the License.
  */
 /*
-* Description  : Test warnings for cardinality hints
-* Expected Res : No warning, honor hint
+* Description  : Test warnings for enhanced hash join hint
+* Expected Res : Warning, ignore hint
 * Date         : 03/09/2023
 */
 // requesttype=application/json
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity n 100.0 */ = n.n_nationkey;
+WHERE c.c_nationkey /*+ productivity 100.0 */ = n.n_nationkey;
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.07.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.10.query.sqlpp
similarity index 100%
copy from 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.07.query.sqlpp
copy to 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.10.query.sqlpp
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.11.query.sqlpp
similarity index 100%
copy from 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
copy to 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.11.query.sqlpp
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.09.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.12.query.sqlpp
similarity index 100%
copy from 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.09.query.sqlpp
copy to 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.12.query.sqlpp
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.13.query.sqlpp
similarity index 94%
copy from 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
copy to 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.13.query.sqlpp
index 6a6a440489..6cc8effa94 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.13.query.sqlpp
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity n 100 */ = n.n_nationkey;
+WHERE c.c_nationkey /*+ productivity n -10.0 */ = n.n_nationkey;
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.14.query.sqlpp
similarity index 94%
copy from 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
copy to 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.14.query.sqlpp
index 6a6a440489..dd56076ebe 100644
--- 
a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.08.query.sqlpp
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/warnings/cardinality-hint-warning/cardinality-hint-warning.14.query.sqlpp
@@ -28,4 +28,4 @@ use tpch;
 
 SELECT count(*)
 FROM customer c, nation n
-WHERE c.c_nationkey /*+ productivity n 100 */ = n.n_nationkey;
+WHERE c.c_nationkey /*+ productivity n 0.0 */ = n.n_nationkey;
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.10.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.10.adm
new file mode 100644
index 0000000000..3ff59f66ac
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.10.adm
@@ -0,0 +1 @@
+{ "$1": 0 }
\ No newline at end of file
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.11.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.11.adm
new file mode 100644
index 0000000000..3ff59f66ac
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.11.adm
@@ -0,0 +1 @@
+{ "$1": 0 }
\ No newline at end of file
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.12.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.12.adm
new file mode 100644
index 0000000000..3ff59f66ac
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.12.adm
@@ -0,0 +1 @@
+{ "$1": 0 }
\ No newline at end of file
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.13.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.13.adm
new file mode 100644
index 0000000000..3ff59f66ac
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.13.adm
@@ -0,0 +1 @@
+{ "$1": 0 }
\ No newline at end of file
diff --git 
a/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.14.adm
 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.14.adm
new file mode 100644
index 0000000000..3ff59f66ac
--- /dev/null
+++ 
b/asterixdb/asterix-app/src/test/resources/runtimets/results/warnings/cardinality-hint-warning/cardinality-hint-warning.14.adm
@@ -0,0 +1 @@
+{ "$1": 0 }
\ 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 f9e2e7030f..bf2d80f5d6 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -15464,11 +15464,15 @@
       <compilation-unit name="cardinality-hint-warning">
         <output-dir compare="Text">cardinality-hint-warning</output-dir>
         <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
selectivity. Expected selectivity value (in line 31, at column 
52)]]></expected-warn>
-        <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
selectivity. Selectivity value has to be in decimal format (in line 31, at 
column 52)]]></expected-warn>
+        <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
selectivity. Selectivity has to be a decimal value greater than 0 and less than 
1 (in line 31, at column 52)]]></expected-warn>
+        <expected-warn>HYR10006: Could not apply selectivity hint: Selectivity 
specified: 0.0, has to be a decimal value greater than 0 and less than 1 (in 
line 31, at column 73)</expected-warn>
+        <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
selectivity. Selectivity has to be a decimal value greater than 0 and less than 
1 (in line 31, at column 52)]]></expected-warn>
         <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
productivity. Expected productivity collection name and value (in line 31, at 
column 23)]]></expected-warn>
         <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
productivity. Invalid format for productivity values (in line 31, at column 
23)]]></expected-warn>
         <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
productivity. Invalid format for productivity values (in line 31, at column 
23)]]></expected-warn>
         <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
productivity. Invalid format for productivity values (in line 31, at column 
23)]]></expected-warn>
+        <expected-warn><![CDATA[ASX1132: Invalid specification for hint 
productivity. Invalid format for productivity values (in line 31, at column 
23)]]></expected-warn>
+        <expected-warn>HYR10006: Could not apply productivity hint: 
Productivity specified: 0.0, has to be a decimal value greater than 0 (in line 
31, at column 47)</expected-warn>
       </compilation-unit>
     </test-case>
     <test-case FilePath="warnings" check-warnings="true">
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj 
b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index 875de45601..c7ded7bcfd 100644
--- a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
+++ b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
@@ -700,7 +700,7 @@ class SQLPPParser extends ScopeChecker implements IParser {
                  selectivity = Double.parseDouble (mat.group());
                }
                else {
-                 throw new SqlppParseException(getSourceLocation(hintToken), 
"Selectivity value has to be in decimal format");
+                 throw new SqlppParseException(getSourceLocation(hintToken), 
"Selectivity has to be a decimal value greater than 0 and less than 1");
                }
                return new PredicateCardinalityAnnotation(selectivity);
              }
@@ -727,7 +727,7 @@ class SQLPPParser extends ScopeChecker implements IParser {
                    productivity = Double.parseDouble (numMat.group());
                  }
                  else {
-                   throw new SqlppParseException(getSourceLocation(hintToken), 
"Productivity value has to be in decimal format");
+                   throw new SqlppParseException(getSourceLocation(hintToken), 
"Productivity has to be a decimal value greater than 0");
                  }
                }
                else {

Reply via email to