Change in asterixdb[neo]: [ASTERIXDB-3135] Cardinality hints enhanced error checking

[AsterixDB Code Review]

>From Vijay Sarathy <vijay.sara...@couchbase.com>:

Vijay Sarathy has uploaded this change for review. ( 
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17420 )


Change subject: [ASTERIXDB-3135] Cardinality hints enhanced error checking
......................................................................

[ASTERIXDB-3135] Cardinality hints enhanced error checking

Change-Id: I2b260845d3122ae31d4dd461e23e131ac064521c
---
M 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/Stats.java
M asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
2 files changed, 39 insertions(+), 4 deletions(-)



  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb 
refs/changes/20/17420/1

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 96bc412..1e34887 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.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,28 @@
             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 value: " + 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,6 +171,13 @@
         PredicateCardinalityAnnotation pca = 
afcExpr.getAnnotation(PredicateCardinalityAnnotation.class);
         if (pca != null) {
             s = pca.getSelectivity();
+            if (s <= 0 || s >= 1) {
+                IWarningCollector warningCollector = 
joinEnum.optCtx.getWarningCollector();
+                if (warningCollector.shouldWarn()) {
+                    
warningCollector.warn(Warning.of(afcExpr.getSourceLocation(), 
ErrorCode.INAPPLICABLE_HINT,
+                            "selectivity", "selectivity value: " + s + " has 
to be a decimal value between 0 and 1"));
+                }
+            }
             sel *= s;
         } else {
             JoinProductivityAnnotation jpa = 
afcExpr.getAnnotation(JoinProductivityAnnotation.class);
diff --git a/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj 
b/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
index 875de45..ec63ea3 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 @@
                  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 between 0 and 1");
                }
                return new PredicateCardinalityAnnotation(selectivity);
              }
@@ -727,7 +727,7 @@
                    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 in decimal value greater than 0");
                  }
                }
                else {

--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17420
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: I2b260845d3122ae31d4dd461e23e131ac064521c
Gerrit-Change-Number: 17420
Gerrit-PatchSet: 1
Gerrit-Owner: Vijay Sarathy <vijay.sara...@couchbase.com>
Gerrit-MessageType: newchange