You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by da...@apache.org on 2019/07/02 12:29:59 UTC

[calcite] 01/01: [CALCITE-3152] Unify throws in sql parser

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

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

commit d9d79ba69056ff69ab69b4c871ca1d83b0bfff15
Author: yuzhao.cyz <yu...@alibaba-inc.com>
AuthorDate: Fri Jun 28 16:01:44 2019 +0800

    [CALCITE-3152] Unify throws in sql parser
    
    Replace generateParseException() and new ParseExecption to
    SqlUtil.newContextException, this makes the error message more user
    friendly.
---
 core/src/main/codegen/templates/Parser.jj          | 29 ++++++++++++----------
 .../apache/calcite/runtime/CalciteResource.java    |  3 +++
 .../calcite/runtime/CalciteResource.properties     |  1 +
 .../apache/calcite/sql/parser/SqlParserTest.java   | 25 +++++++++++--------
 4 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
index f2c77ae..2ff4c72 100644
--- a/core/src/main/codegen/templates/Parser.jj
+++ b/core/src/main/codegen/templates/Parser.jj
@@ -598,7 +598,7 @@ SqlNode OrderedQueryOrExpr(ExprContext exprContext) :
             start = UnsignedNumericLiteralOrParam()
             <COMMA> count = UnsignedNumericLiteralOrParam() {
                 if (!this.conformance.isLimitStartCountAllowed()) {
-                    throw new ParseException(RESOURCE.limitStartCountNotAllowed().str());
+                    throw SqlUtil.newContextException(getPos(), RESOURCE.limitStartCountNotAllowed());
                 }
             }
         |
@@ -1787,7 +1787,7 @@ SqlNode FromClause() :
             <CROSS> { joinType = JoinType.CROSS.symbol(getPos()); } <APPLY>
             e2 = TableRef2(true) {
                 if (!this.conformance.isApplyAllowed()) {
-                    throw new ParseException(RESOURCE.applyNotAllowed().str());
+                    throw SqlUtil.newContextException(getPos(), RESOURCE.applyNotAllowed());
                 }
                 e = new SqlJoin(joinType.getParserPosition(),
                     e,
@@ -1801,7 +1801,7 @@ SqlNode FromClause() :
             <OUTER> { joinType = JoinType.LEFT.symbol(getPos()); } <APPLY>
             e2 = TableRef2(true) {
                 if (!this.conformance.isApplyAllowed()) {
-                    throw new ParseException(RESOURCE.applyNotAllowed().str());
+                    throw SqlUtil.newContextException(getPos(), RESOURCE.applyNotAllowed());
                 }
                 e = new SqlJoin(joinType.getParserPosition(),
                     e,
@@ -1974,7 +1974,7 @@ SqlNode TableRef2(boolean lateral) :
                 if (rate.compareTo(BigDecimal.ZERO) < 0
                     || rate.compareTo(ONE_HUNDRED) > 0)
                 {
-                    throw new ParseException(RESOURCE.invalidSampleSize().str());
+                    throw SqlUtil.newContextException(getPos(), RESOURCE.invalidSampleSize());
                 }
 
                 // Treat TABLESAMPLE(0) and TABLESAMPLE(100) as no table
@@ -2054,7 +2054,7 @@ void CompoundIdentifierType(List<SqlNode> list, List<SqlNode> extendList) :
     [
         type = DataType() {
             if (!this.conformance.allowExtend()) {
-                throw new ParseException(RESOURCE.extendNotAllowed().str());
+                throw SqlUtil.newContextException(getPos(), RESOURCE.extendNotAllowed());
             }
         }
         [
@@ -3277,7 +3277,7 @@ SqlKind comp() :
 |
     <NE2> {
         if (!this.conformance.isBangEqualAllowed()) {
-            throw new ParseException(RESOURCE.bangEqualNotAllowed().str());
+            throw SqlUtil.newContextException(getPos(), RESOURCE.bangEqualNotAllowed());
         }
         return SqlKind.NOT_EQUALS;
     }
@@ -4491,7 +4491,8 @@ int UnsignedIntLiteral() :
         try {
             return Integer.parseInt(t.image);
         } catch (NumberFormatException ex) {
-            throw generateParseException();
+            throw SqlUtil.newContextException(getPos(),
+                RESOURCE.invalidLiteral(t.image, Integer.class.getCanonicalName()));
         }
     }
 }
@@ -4510,7 +4511,8 @@ int IntLiteral() :
         try {
             return Integer.parseInt(t.image);
         } catch (NumberFormatException ex) {
-            throw generateParseException();
+            throw SqlUtil.newContextException(getPos(),
+                RESOURCE.invalidLiteral(t.image, Integer.class.getCanonicalName()));
         }
     }
 |
@@ -4518,7 +4520,8 @@ int IntLiteral() :
         try {
             return -Integer.parseInt(t.image);
         } catch (NumberFormatException ex) {
-            throw generateParseException();
+            throw SqlUtil.newContextException(getPos(),
+                RESOURCE.invalidLiteral(t.image, Integer.class.getCanonicalName()));
         }
     }
 }
@@ -4624,7 +4627,7 @@ SqlTypeName SqlTypeName(Span s) :
 |
     <GEOMETRY> {
         if (!this.conformance.allowGeometry()) {
-            throw new ParseException(RESOURCE.geometryDisabled().str());
+            throw SqlUtil.newContextException(getPos(), RESOURCE.geometryDisabled());
         }
         return SqlTypeName.GEOMETRY;
     }
@@ -6038,7 +6041,7 @@ SqlBinaryOperator BinaryQueryOperator() :
         |
             <SET_MINUS> {
                 if (!this.conformance.isMinusAllowed()) {
-                    throw new ParseException(RESOURCE.minusNotAllowed().str());
+                    throw SqlUtil.newContextException(getPos(), RESOURCE.minusNotAllowed());
                 }
             }
         )
@@ -6100,7 +6103,7 @@ SqlBinaryOperator BinaryRowOperator() :
 |   <NE> { return SqlStdOperatorTable.NOT_EQUALS; }
 |   <NE2> {
         if (!this.conformance.isBangEqualAllowed()) {
-            throw new ParseException(RESOURCE.bangEqualNotAllowed().str());
+            throw SqlUtil.newContextException(getPos(), RESOURCE.bangEqualNotAllowed());
         }
         return SqlStdOperatorTable.NOT_EQUALS;
     }
@@ -6110,7 +6113,7 @@ SqlBinaryOperator BinaryRowOperator() :
 |   <SLASH> { return SqlStdOperatorTable.DIVIDE; }
 |   <PERCENT_REMAINDER> {
         if (!this.conformance.isPercentRemainderAllowed()) {
-            throw new ParseException(RESOURCE.percentRemainderNotAllowed().str());
+            throw SqlUtil.newContextException(getPos(), RESOURCE.percentRemainderNotAllowed());
         }
         return SqlStdOperatorTable.PERCENT_REMAINDER;
     }
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index 8f1c6c6..c830eb2 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -99,6 +99,9 @@ public interface CalciteResource {
   @Property(name = "SQLSTATE", value = "2202H")
   ExInst<CalciteException> invalidSampleSize();
 
+  @BaseMessage("Literal ''{0}'' can not be parsed to type ''{1}''")
+  ExInst<CalciteException> invalidLiteral(String a0, String a1);
+
   @BaseMessage("Unknown character set ''{0}''")
   ExInst<CalciteException> unknownCharacterSet(String a0);
 
diff --git a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index 0a01778..d77353a 100644
--- a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++ b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -126,6 +126,7 @@ ColumnCountMismatchInSetop=Column count mismatch in {0}
 ColumnTypeMismatchInSetop=Type mismatch in column {0,number} of {1}
 BinaryLiteralOdd=Binary literal string must contain an even number of hexits
 BinaryLiteralInvalid=Binary literal string must contain only characters ''0'' - ''9'', ''A'' - ''F''
+InvalidLiteral=Literal ''{0}'' can not be parsed to type ''{1}''
 UnsupportedIntervalLiteral=Illegal interval literal format {0} for {1}
 IntervalFieldExceedsPrecision=Interval field value {0,number} exceeds precision of {1} field
 CompoundOrderByProhibitsRange=RANGE clause cannot be used with compound ORDER BY clause
diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
index 3160d9c..d7e18e0 100644
--- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -976,7 +976,7 @@ public class SqlParserTest {
     //   you that != is SQL's not-equals operator; those texts are false;
     //   it's one of those unstampoutable urban myths."
     // Therefore, we only support != with certain SQL conformance levels.
-    checkExpFails("'abc'!=123",
+    checkExpFails("'abc'^!=^123",
         "Bang equal '!=' is not allowed under the current SQL conformance level");
   }
 
@@ -2227,8 +2227,9 @@ public class SqlParserTest {
   @Test public void testSetMinus() {
     final String pattern =
         "MINUS is not allowed under the current SQL conformance level";
-    final String sql = "select col1 from table1 MINUS select col1 from table2";
-    sql(sql).fails(pattern);
+    final String sql0 = "select col1 from table1 ^MINUS^ select col1 from table2";
+    final String sql1 = "select col1 from table1 MINUS select col1 from table2";
+    sql(sql0).fails(pattern);
 
     conformance = SqlConformanceEnum.ORACLE_10;
     final String expected = "(SELECT `COL1`\n"
@@ -2236,7 +2237,7 @@ public class SqlParserTest {
         + "EXCEPT\n"
         + "SELECT `COL1`\n"
         + "FROM `TABLE2`)";
-    sql(sql).ok(expected);
+    sql(sql1).ok(expected);
 
     final String sql2 =
         "select col1 from table1 MINUS ALL select col1 from table2";
@@ -2448,22 +2449,24 @@ public class SqlParserTest {
   @Test public void testApply() {
     final String pattern =
         "APPLY operator is not allowed under the current SQL conformance level";
-    final String sql = "select * from dept\n"
+    final String sql0 = "select * from dept\n"
+        + "cross apply table(ramp(deptno)) as t(a^)^";
+    final String sql1 = "select * from dept\n"
         + "cross apply table(ramp(deptno)) as t(a)";
-    sql(sql).fails(pattern);
+    sql(sql0).fails(pattern);
 
     conformance = SqlConformanceEnum.SQL_SERVER_2008;
     final String expected = "SELECT *\n"
         + "FROM `DEPT`\n"
         + "CROSS JOIN LATERAL TABLE(`RAMP`(`DEPTNO`)) AS `T` (`A`)";
-    sql(sql).ok(expected);
+    sql(sql1).ok(expected);
 
     // Supported in Oracle 12 but not Oracle 10
     conformance = SqlConformanceEnum.ORACLE_10;
-    sql(sql).fails(pattern);
+    sql(sql0).fails(pattern);
 
     conformance = SqlConformanceEnum.ORACLE_12;
-    sql(sql).ok(expected);
+    sql(sql1).ok(expected);
   }
 
   /** Tests OUTER APPLY. */
@@ -2824,7 +2827,7 @@ public class SqlParserTest {
     conformance = SqlConformanceEnum.DEFAULT;
     final String error = "'LIMIT start, count' is not allowed under the "
         + "current SQL conformance level";
-    sql("select a from foo limit 1,2")
+    sql("select a from foo limit 1,^2^")
         .fails(error);
 
     // "limit all" is equivalent to no limit
@@ -7135,7 +7138,7 @@ public class SqlParserTest {
   }
 
   @Test public void testGeometry() {
-    checkExpFails("cast(null as geometry)",
+    checkExpFails("cast(null as ^geometry^)",
         "Geo-spatial extensions and the GEOMETRY data type are not enabled");
     conformance = SqlConformanceEnum.LENIENT;
     checkExp("cast(null as geometry)", "CAST(NULL AS GEOMETRY)");