[CALCITE-2542] In SQL parser, allow '. field' to follow expressions other than tables and columns (Rong Rong)
Fix how SqlNode AtomicRowExpression + DOT operation is parsed, adding in <DOT> parser for expression2b for parsing additional identifier after AtomicRowExpression. Close apache/calcite#933 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/45258404 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/45258404 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/45258404 Branch: refs/heads/master Commit: 45258404b7844204266f8c356a8e41ef1f1e3683 Parents: 849f141 Author: Rong Rong <[email protected]> Authored: Mon Nov 19 16:42:10 2018 -0800 Committer: Julian Hyde <[email protected]> Committed: Fri Nov 30 20:29:36 2018 -0800 ---------------------------------------------------------------------- core/src/main/codegen/templates/Parser.jj | 10 ++++++++ .../apache/calcite/sql/fun/SqlDotOperator.java | 18 +++++++++++--- .../calcite/sql/parser/SqlParserTest.java | 15 +++++++---- .../calcite/test/SqlToRelConverterTest.java | 10 ++++++++ .../apache/calcite/test/SqlValidatorTest.java | 26 ++++++++++++++++++-- .../calcite/test/SqlToRelConverterTest.xml | 22 +++++++++++++++++ 6 files changed, 90 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/main/codegen/templates/Parser.jj ---------------------------------------------------------------------- diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj index 247c8b0..b137d6f 100644 --- a/core/src/main/codegen/templates/Parser.jj +++ b/core/src/main/codegen/templates/Parser.jj @@ -3000,6 +3000,7 @@ void Expression2b(ExprContext exprContext, List<Object> list) : { SqlNode e; SqlOperator op; + String p; } { ( @@ -3011,6 +3012,15 @@ void Expression2b(ExprContext exprContext, List<Object> list) : e = Expression3(exprContext) { list.add(e); } + ( + <DOT> + p = Identifier() { + list.add( + new SqlParserUtil.ToTreeListItem( + SqlStdOperatorTable.DOT, getPos())); + list.add(new SqlIdentifier(p, getPos())); + } + )* } /** http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java index e9b70c7..3739a40 100644 --- a/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java +++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlDotOperator.java @@ -93,14 +93,22 @@ public class SqlDotOperator extends SqlSpecialOperator { @Override public RelDataType deriveType(SqlValidator validator, SqlValidatorScope scope, SqlCall call) { - RelDataType nodeType = validator.deriveType(scope, call.getOperandList().get(0)); + final SqlNode operand = call.getOperandList().get(0); + final RelDataType nodeType = + validator.deriveType(scope, operand); assert nodeType != null; + if (!nodeType.isStruct()) { + throw SqlUtil.newContextException(operand.getParserPosition(), + Static.RESOURCE.incompatibleTypes()); + } - final String fieldName = call.getOperandList().get(1).toString(); - RelDataTypeField field = + final SqlNode fieldId = call.operand(1); + final String fieldName = fieldId.toString(); + final RelDataTypeField field = nodeType.getField(fieldName, false, false); if (field == null) { - throw SqlUtil.newContextException(SqlParserPos.ZERO, Static.RESOURCE.unknownField(fieldName)); + throw SqlUtil.newContextException(fieldId.getParserPosition(), + Static.RESOURCE.unknownField(fieldName)); } RelDataType type = field.getType(); @@ -130,6 +138,8 @@ public class SqlDotOperator extends SqlSpecialOperator { callBinding.getValidator().deriveType(callBinding.getScope(), left); if (type.getSqlTypeName() != SqlTypeName.ROW) { return false; + } else if (type.getSqlIdentifier().isStar()) { + return false; } final RelDataType operandType = callBinding.getOperandType(0); final SqlSingleOperandTypeChecker checker = getChecker(operandType); http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java ---------------------------------------------------------------------- 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 535aa15..a7a1233 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 @@ -1094,6 +1094,11 @@ public class SqlParserTest { return false; } + @Test public void testRowWitDot() { + check("select (1,2).a from c.t", "SELECT ((ROW(1, 2)).`A`)\nFROM `C`.`T`"); + check("select row(1,2).a from c.t", "SELECT ((ROW(1, 2)).`A`)\nFROM `C`.`T`"); + } + @Test public void testPeriod() { // We don't have a PERIOD constructor currently; // ROW constructor is sufficient for now. @@ -1514,6 +1519,10 @@ public class SqlParserTest { + "FROM `EMP`"); } + @Test public void testFunctionCallWithDot() { + checkExp("foo(a,b).c", "(`FOO`(`A`, `B`).`C`)"); + } + @Test public void testFunctionInFunction() { checkExp("ln(power(2,2))", "LN(POWER(2, 2))"); } @@ -2472,11 +2481,6 @@ public class SqlParserTest { + "FROM `EMP`"); } - @Test public void testTableStarColumnFails() { - sql("select emp.*^.^xx from emp") - .fails("(?s).*Encountered \".\" .*"); - } - @Test public void testNotExists() { check( "select * from dept where not not exists (select * from emp) and true", @@ -6620,6 +6624,7 @@ public class SqlParserTest { "(?s)Encountered \"to year\" at line 1, column 19.\n" + "Was expecting one of:\n" + " <EOF> \n" + + " \"\\.\" \\.\\.\\.\n" + " \"NOT\" \\.\\.\\..*"); checkExpFails("interval '1-2' year ^to^ day", ANY); checkExpFails("interval '1-2' year ^to^ hour", ANY); http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java index 94707fe..77fa01c 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -78,6 +78,16 @@ public class SqlToRelConverterTest extends SqlToRelTestBase { sql(sql).convertsTo(plan); } + @Test public void testDotLiteralAfterNestedRow() { + final String sql = "select ((1,2),(3,4,5)).\"EXPR$1\".\"EXPR$2\" from emp"; + sql(sql).ok(); + } + + @Test public void testDotLiteralAfterRow() { + final String sql = "select row(1,2).\"EXPR$1\" from emp"; + sql(sql).ok(); + } + @Test public void testIntegerLiteral() { final String sql = "select 1 from emp"; sql(sql).ok(); http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java index ab2d444..c59f3a1 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -1259,6 +1259,26 @@ public class SqlValidatorTest extends SqlValidatorTestCase { "INTEGER NOT NULL"); } + @Test public void testRowWitValidDot() { + checkColumnType("select ((1,2),(3,4,5)).\"EXPR$1\".\"EXPR$2\"\n from dept", + "INTEGER NOT NULL"); + checkColumnType("select row(1,2).\"EXPR$1\" from dept", + "INTEGER NOT NULL"); + checkColumnType("select t.a.\"EXPR$1\" from (select row(1,2) as a from (values (1))) as t", + "INTEGER NOT NULL"); + } + + @Test public void testRowWithInvalidDotOperation() { + final String sql = "select t.^s.\"EXPR$1\"^ from (\n" + + " select 1 AS s from (values (1))) as t"; + checkExpFails(sql, + "(?s).*Column 'S\\.EXPR\\$1' not found in table 'T'.*"); + checkExpFails("select ^array[1, 2, 3]^.\"EXPR$1\" from dept", + "(?s).*Incompatible types.*"); + checkExpFails("select ^'mystr'^.\"EXPR$1\" from dept", + "(?s).*Incompatible types.*"); + } + @Test public void testMultiset() { checkExpType("multiset[1]", "INTEGER NOT NULL MULTISET NOT NULL"); checkExpType( @@ -4818,8 +4838,10 @@ public class SqlValidatorTest extends SqlValidatorTestCase { @Test public void testStarDotIdFails() { // Fails in parser + sql("select emp.^*^.\"EXPR$1\" from emp") + .fails("(?s).*Unknown field '\\*'"); sql("select emp.^*^.foo from emp") - .fails("(?s).*Encountered \".\" at .*"); + .fails("(?s).*Unknown field '\\*'"); // Parser does not allow star dot identifier. sql("select ^*^.foo from emp") .fails("(?s).*Encountered \".\" at .*"); @@ -7966,7 +7988,7 @@ public class SqlValidatorTest extends SqlValidatorTestCase { } @Test public void testArrayOfRecordType() { - sql("SELECT name, dept_nested.employees[1].ne as ne from dept_nested") + sql("SELECT name, dept_nested.employees[1].^ne^ as ne from dept_nested") .fails("Unknown field 'NE'"); sql("SELECT name, dept_nested.employees[1].ename as ename from dept_nested") .type("RecordType(VARCHAR(10) NOT NULL NAME, VARCHAR(10) ENAME) NOT NULL"); http://git-wip-us.apache.org/repos/asf/calcite/blob/45258404/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml ---------------------------------------------------------------------- diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml index c218e7c..9128946 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -717,6 +717,28 @@ LogicalProject(EXPR$0=[CHAR_LENGTH('foo')]) <![CDATA[values (character_length('foo'))]]> </Resource> </TestCase> + <TestCase name="testDotLiteralAfterNestedRow"> + <Resource name="sql"> + <![CDATA[select ((1,2),(3,4,5))."EXPR$1"."EXPR$2" from emp]]> + </Resource> + <Resource name="plan"> + <![CDATA[ +LogicalProject(EXPR$0=[ROW(ROW(1, 2), ROW(3, 4, 5)).EXPR$1.EXPR$2]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> + <TestCase name="testDotLiteralAfterRow"> + <Resource name="sql"> + <![CDATA[select row(1,2)."EXPR$1" from emp]]> + </Resource> + <Resource name="plan"> + <![CDATA[ +LogicalProject(EXPR$0=[ROW(1, 2).EXPR$1]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> <TestCase name="testDynamicNestedColumn"> <Resource name="sql"> <![CDATA[select t3.fake_q1['fake_col2'] as fake2
