Repository: calcite Updated Branches: refs/heads/master 6679353f2 -> 93a96ddf4
[CALCITE-1889] Accept compound identifiers in SqlValidatorUtil.checkIdentifierListForDuplicates() (Rajeshbabu Chintaguntla) Close apache/calcite#487 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/93a96ddf Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/93a96ddf Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/93a96ddf Branch: refs/heads/master Commit: 93a96ddf4adf688d5b30a3e3224805d32d3d54bc Parents: 6679353 Author: Rajeshbabu Chintaguntla <[email protected]> Authored: Wed Jun 28 23:34:14 2017 +0530 Committer: Julian Hyde <[email protected]> Committed: Fri Jul 14 11:39:48 2017 -0700 ---------------------------------------------------------------------- .../calcite/sql/validate/SqlValidatorUtil.java | 10 ++--- .../sql/validate/SqlValidatorUtilTest.java | 29 ++++++++++++++ .../apache/calcite/test/MockCatalogReader.java | 22 ++++++++++- .../apache/calcite/test/SqlValidatorTest.java | 41 +++++++++++++------- .../calcite/test/SqlValidatorTestCase.java | 33 +++++++++------- 5 files changed, 102 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/93a96ddf/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java index c64e93d..f12c1d9 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java @@ -266,16 +266,16 @@ public class SqlValidatorUtil { */ static void checkIdentifierListForDuplicates(List<SqlNode> columnList, SqlValidatorImpl.ValidationErrorFunction validationErrorFunction) { - final List<String> names = Lists.transform(columnList, - new Function<SqlNode, String>() { - public String apply(SqlNode o) { - return ((SqlIdentifier) o).getSimple(); + final List<List<String>> names = Lists.transform(columnList, + new Function<SqlNode, List<String>>() { + public List<String> apply(SqlNode o) { + return ((SqlIdentifier) o).names; } }); final int i = Util.firstDuplicate(names); if (i >= 0) { throw validationErrorFunction.apply(columnList.get(i), - RESOURCE.duplicateNameInColumnList(names.get(i))); + RESOURCE.duplicateNameInColumnList(Util.last(names.get(i)))); } } http://git-wip-us.apache.org/repos/asf/calcite/blob/93a96ddf/core/src/test/java/org/apache/calcite/sql/validate/SqlValidatorUtilTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/sql/validate/SqlValidatorUtilTest.java b/core/src/test/java/org/apache/calcite/sql/validate/SqlValidatorUtilTest.java index 5fe474f..69a8249 100644 --- a/core/src/test/java/org/apache/calcite/sql/validate/SqlValidatorUtilTest.java +++ b/core/src/test/java/org/apache/calcite/sql/validate/SqlValidatorUtilTest.java @@ -16,11 +16,19 @@ */ package org.apache.calcite.sql.validate; +import org.apache.calcite.runtime.CalciteContextException; +import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.test.DefaultSqlTestFactory; +import org.apache.calcite.sql.test.SqlTesterImpl; + import com.google.common.collect.Lists; import org.junit.Test; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Locale; @@ -29,6 +37,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.sameInstance; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; /** * Tests for {@link SqlValidatorUtil}. @@ -109,6 +118,26 @@ public class SqlValidatorUtilTest { checkChangedFieldList(nameList, resultList, false); } + @SuppressWarnings("resource") + @Test public void testCheckingDuplicatesWithCompoundIdentifiers() { + final List<SqlNode> newList = new ArrayList<>(2); + newList.add(new SqlIdentifier(Arrays.asList("f0", "c0"), SqlParserPos.ZERO)); + newList.add(new SqlIdentifier(Arrays.asList("f0", "c0"), SqlParserPos.ZERO)); + final SqlTesterImpl tester = + new SqlTesterImpl(DefaultSqlTestFactory.INSTANCE); + final SqlValidatorImpl validator = + (SqlValidatorImpl) tester.getValidator(); + try { + SqlValidatorUtil.checkIdentifierListForDuplicates(newList, + validator.getValidationErrorFunction()); + fail("expected exception"); + } catch (CalciteContextException e) { + // ok + } + // should not throw + newList.set(1, new SqlIdentifier(Arrays.asList("f0", "c1"), SqlParserPos.ZERO)); + SqlValidatorUtil.checkIdentifierListForDuplicates(newList, null); + } } // End SqlValidatorUtilTest.java http://git-wip-us.apache.org/repos/asf/calcite/blob/93a96ddf/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java b/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java index 59812d7..b77dd6f 100644 --- a/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java +++ b/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java @@ -527,7 +527,6 @@ public class MockCatalogReader extends CalciteCatalogReader { * SQL needs to be parsed) and so are not used for all tests. */ public MockCatalogReader init2() { MockSchema salesSchema = new MockSchema("SALES"); - // Same as "EMP_20" except it uses ModifiableViewTable which populates // constrained columns with default values on INSERT and has a single constraint on DEPTNO. List<String> empModifiableViewNames = ImmutableList.of( @@ -573,6 +572,27 @@ public class MockCatalogReader extends CalciteCatalogReader { empModifiableViewNames3.get(2), false, 20, null); registerTable(mockEmpViewTable3); + MockSchema structTypeSchema = new MockSchema("STRUCT"); + registerSchema(structTypeSchema); + final Fixture f = new Fixture(); + final List<CompoundNameColumn> columnsExtended = Arrays.asList( + new CompoundNameColumn("", "K0", f.varchar20TypeNull), + new CompoundNameColumn("", "C1", f.varchar20TypeNull), + new CompoundNameColumn("F0", "C0", f.intType), + new CompoundNameColumn("F1", "C1", f.intTypeNull)); + final List<CompoundNameColumn> extendedColumns = + new ArrayList<CompoundNameColumn>(columnsExtended); + extendedColumns.add(new CompoundNameColumn("F2", "C2", f.varchar20Type)); + final CompoundNameColumnResolver structExtendedTableResolver = + new CompoundNameColumnResolver(extendedColumns, "F0"); + final MockTable structExtendedTypeTable = + MockTable.create(this, structTypeSchema, "T_EXTEND", false, 100, + structExtendedTableResolver); + for (CompoundNameColumn column : columnsExtended) { + structExtendedTypeTable.addColumn(column.getName(), column.type); + } + registerTable(structExtendedTypeTable); + return this; } http://git-wip-us.apache.org/repos/asf/calcite/blob/93a96ddf/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 ae1ee37..224bb25 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -8707,22 +8707,37 @@ public class SqlValidatorTest extends SqlValidatorTestCase { @Test public void testInsertWithExtendedColumns() { - final SqlTester lenient = - tester.withConformance(SqlConformanceEnum.LENIENT); - final SqlTester strict = - tester.withConformance(SqlConformanceEnum.STRICT_2003); + final String sql0 = "insert into empnullables\n" + + " (empno, ename, \"f.dc\" varchar(10))\n" + + "values (?, ?, ?)"; + sql(sql0) + .tester(EXTENDED_CATALOG_TESTER_LENIENT) + .ok() + .bindType("RecordType(INTEGER ?0, VARCHAR(20) ?1, VARCHAR(10) ?2)") + .tester(EXTENDED_CATALOG_TESTER_2003) + .fails("Extended columns not allowed under " + + "the current SQL conformance level"); - String sql0 = "insert into empnullables (empno, ename, \"f.dc\" varchar(10))\n" - + "values (?, ?, ?)"; - sql(sql0).tester(lenient).ok() - .bindType("RecordType(INTEGER ?0, VARCHAR(20) ?1, VARCHAR(10) ?2)") - .tester(strict).fails("Extended columns not allowed under " - + "the current SQL conformance level"); - sql0 = "insert into empnullables (empno, ename, dynamic_column double not null)\n" + final String sql1 = "insert into empnullables\n" + + " (empno, ename, dynamic_column double not null)\n" + "values (?, ?, ?)"; - sql(sql0).tester(lenient).ok() + sql(sql1) + .tester(EXTENDED_CATALOG_TESTER_LENIENT) + .ok() .bindType("RecordType(INTEGER ?0, VARCHAR(20) ?1, DOUBLE ?2)") - .tester(strict).fails("Extended columns not allowed under " + .tester(EXTENDED_CATALOG_TESTER_2003) + .fails("Extended columns not allowed under " + + "the current SQL conformance level"); + + final String sql2 = "insert into struct.t_extend\n" + + " (f0.c0, f1.c1, \"F2\".\"C2\" varchar(20) not null)\n" + + "values (?, ?, ?)"; + sql(sql2) + .tester(EXTENDED_CATALOG_TESTER_LENIENT) + .ok() + .bindType("RecordType(INTEGER ?0, INTEGER ?1, VARCHAR(20) ?2)") + .tester(EXTENDED_CATALOG_TESTER_2003) + .fails("Extended columns not allowed under " + "the current SQL conformance level"); } http://git-wip-us.apache.org/repos/asf/calcite/blob/93a96ddf/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java index 12ae488..554c184 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java @@ -65,23 +65,24 @@ public class SqlValidatorTestCase { Pattern.compile( "(?s)From line ([0-9]+), column ([0-9]+) to line ([0-9]+), column ([0-9]+): (.*)"); + private static final SqlTestFactory EXTENDED_TEST_FACTORY = + new DelegatingSqlTestFactory(DefaultSqlTestFactory.INSTANCE) { + @Override public MockCatalogReader createCatalogReader( + SqlTestFactory factory, JavaTypeFactory typeFactory) { + return super.createCatalogReader(this, typeFactory).init2(); + } + }; + static final SqlTesterImpl EXTENDED_CATALOG_TESTER = - new SqlTesterImpl( - new DelegatingSqlTestFactory(DefaultSqlTestFactory.INSTANCE) { - @Override public MockCatalogReader createCatalogReader( - SqlTestFactory factory, JavaTypeFactory typeFactory) { - return super.createCatalogReader(this, typeFactory).init2(); - } - }); + new SqlTesterImpl(EXTENDED_TEST_FACTORY); static final SqlTesterImpl EXTENDED_CATALOG_TESTER_2003 = - new SqlTesterImpl( - new DelegatingSqlTestFactory(DefaultSqlTestFactory.INSTANCE) { - @Override public MockCatalogReader createCatalogReader( - SqlTestFactory factory, JavaTypeFactory typeFactory) { - return super.createCatalogReader(this, typeFactory).init2(); - } - }).withConformance(SqlConformanceEnum.PRAGMATIC_2003); + new SqlTesterImpl(EXTENDED_TEST_FACTORY) + .withConformance(SqlConformanceEnum.PRAGMATIC_2003); + + static final SqlTesterImpl EXTENDED_CATALOG_TESTER_LENIENT = + new SqlTesterImpl(EXTENDED_TEST_FACTORY) + .withConformance(SqlConformanceEnum.LENIENT); //~ Instance fields -------------------------------------------------------- @@ -575,6 +576,10 @@ public class SqlValidatorTestCase { return tester(EXTENDED_CATALOG_TESTER_2003); } + Sql withExtendedCatalogLenient() { + return tester(EXTENDED_CATALOG_TESTER_LENIENT); + } + Sql ok() { tester.assertExceptionIsThrown(sql, null); return this;
