[CALCITE-2084] SqlValidatorImpl.findTable() method incorrectly handles table schema with few schema levels (Volodymyr Vysotskyi)
Close apache/calcite#580 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/26b4712d Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/26b4712d Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/26b4712d Branch: refs/heads/master Commit: 26b4712d4ac36c49aa8c80b2f0841be37c9de1df Parents: 04cbdb2 Author: Volodymyr Vysotskyi <[email protected]> Authored: Sun Dec 10 14:08:47 2017 +0200 Committer: Julian Hyde <[email protected]> Committed: Mon Dec 11 14:24:53 2017 -0800 ---------------------------------------------------------------------- .../calcite/prepare/CalciteCatalogReader.java | 65 ++++------------ .../calcite/sql/validate/SqlValidatorImpl.java | 13 +--- .../calcite/sql/validate/SqlValidatorUtil.java | 79 ++++++++++++++++++++ .../apache/calcite/sql/test/SqlAdvisorTest.java | 4 +- .../apache/calcite/test/MockCatalogReader.java | 33 +++++++- .../apache/calcite/test/SqlValidatorTest.java | 3 + 6 files changed, 130 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/26b4712d/core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java b/core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java index b7f32e1..af0d3a6 100644 --- a/core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java +++ b/core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java @@ -114,36 +114,7 @@ public class CalciteCatalogReader implements Prepare.CatalogReader { public Prepare.PreparingTable getTable(final List<String> names) { // First look in the default schema, if any. // If not found, look in the root schema. - for (List<String> schemaPath : schemaPaths) { - Prepare.PreparingTable table = - getTableFrom(names, schemaPath, nameMatcher); - if (table != null) { - return table; - } - } - return null; - } - - @Override public CalciteConnectionConfig getConfig() { - return config; - } - - private Prepare.PreparingTable getTableFrom(List<String> names, - List<String> schemaNames, - SqlNameMatcher nameMatcher) { - CalciteSchema schema = - getSchema(Iterables.concat(schemaNames, Util.skipLast(names)), - nameMatcher); - if (schema == null) { - return null; - } - final String name = Util.last(names); - CalciteSchema.TableEntry entry = - schema.getTable(name, nameMatcher.isCaseSensitive()); - if (entry == null) { - entry = schema.getTableBasedOnNullaryFunction(name, - nameMatcher.isCaseSensitive()); - } + CalciteSchema.TableEntry entry = SqlValidatorUtil.getTableEntry(this, names); if (entry != null) { final Table table = entry.getTable(); if (table instanceof Wrapper) { @@ -153,12 +124,16 @@ public class CalciteCatalogReader implements Prepare.CatalogReader { return relOptTable; } } - return RelOptTableImpl.create(this, table.getRowType(typeFactory), entry, - null); + return RelOptTableImpl.create(this, + table.getRowType(typeFactory), entry, null); } return null; } + @Override public CalciteConnectionConfig getConfig() { + return config; + } + private Collection<Function> getFunctionsFrom(List<String> names) { final List<Function> functions2 = Lists.newArrayList(); final List<List<String>> schemaNameList = new ArrayList<>(); @@ -172,7 +147,8 @@ public class CalciteCatalogReader implements Prepare.CatalogReader { } } else { for (List<String> schemaPath : schemaPaths) { - CalciteSchema schema = getSchema(schemaPath, nameMatcher); + CalciteSchema schema = + SqlValidatorUtil.getSchema(rootSchema, schemaPath, nameMatcher); if (schema != null) { schemaNameList.addAll(schema.getPath()); } @@ -180,8 +156,8 @@ public class CalciteCatalogReader implements Prepare.CatalogReader { } for (List<String> schemaNames : schemaNameList) { CalciteSchema schema = - getSchema(Iterables.concat(schemaNames, Util.skipLast(names)), - nameMatcher); + SqlValidatorUtil.getSchema(rootSchema, + Iterables.concat(schemaNames, Util.skipLast(names)), nameMatcher); if (schema != null) { final String name = Util.last(names); functions2.addAll(schema.getFunctions(name, true)); @@ -190,28 +166,13 @@ public class CalciteCatalogReader implements Prepare.CatalogReader { return functions2; } - private CalciteSchema getSchema(Iterable<String> schemaNames, - SqlNameMatcher nameMatcher) { - CalciteSchema schema = rootSchema; - for (String schemaName : schemaNames) { - if (schema == rootSchema - && nameMatcher.matches(schemaName, schema.getName())) { - continue; - } - schema = schema.getSubSchema(schemaName, nameMatcher.isCaseSensitive()); - if (schema == null) { - return null; - } - } - return schema; - } - public RelDataType getNamedType(SqlIdentifier typeName) { return null; } public List<SqlMoniker> getAllSchemaObjectNames(List<String> names) { - final CalciteSchema schema = getSchema(names, nameMatcher); + final CalciteSchema schema = + SqlValidatorUtil.getSchema(rootSchema, names, nameMatcher); if (schema == null) { return ImmutableList.of(); } http://git-wip-us.apache.org/repos/asf/calcite/blob/26b4712d/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java index 7c7c065..652bbe9 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java @@ -3440,17 +3440,8 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { catalogReader.nameMatcher().isCaseSensitive()); } - String schemaName = names.get(0); - String tableName = names.get(1); - - CalciteSchema schema = catalogReader.getRootSchema().getSubSchemaMap().get(schemaName); - - if (schema == null) { - return null; - } - - CalciteSchema.TableEntry entry = schema.getTable(tableName, - catalogReader.nameMatcher().isCaseSensitive()); + CalciteSchema.TableEntry entry = + SqlValidatorUtil.getTableEntry(catalogReader, names); return entry == null ? null : entry.getTable(); } http://git-wip-us.apache.org/repos/asf/calcite/blob/26b4712d/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 2200e01..8583cb1 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 @@ -16,6 +16,7 @@ */ package org.apache.calcite.sql.validate; +import org.apache.calcite.jdbc.CalciteSchema; import org.apache.calcite.linq4j.Linq4j; import org.apache.calcite.linq4j.Ord; import org.apache.calcite.plan.RelOptSchemaWithSampling; @@ -51,6 +52,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -931,6 +933,83 @@ public class SqlValidatorUtil { } /** + * Finds a {@link org.apache.calcite.jdbc.CalciteSchema.TableEntry} in a + * given catalog reader whose table has the given name, possibly qualified. + * + * <p>Uses the case-sensitivity policy of the specified catalog reader. + * + * <p>If not found, returns null. + * + * @param catalogReader accessor to the table metadata + * @param names Name of table, may be qualified or fully-qualified + * + * @return TableEntry with a table with the given name, or null + */ + public static CalciteSchema.TableEntry getTableEntry( + SqlValidatorCatalogReader catalogReader, List<String> names) { + // First look in the default schema, if any. + // If not found, look in the root schema. + for (List<String> schemaPath : catalogReader.getSchemaPaths()) { + CalciteSchema schema = + getSchema(catalogReader.getRootSchema(), + Iterables.concat(schemaPath, Util.skipLast(names)), + catalogReader.nameMatcher()); + if (schema == null) { + continue; + } + CalciteSchema.TableEntry entry = + getTableEntryFrom(schema, Util.last(names), + catalogReader.nameMatcher().isCaseSensitive()); + if (entry != null) { + return entry; + } + } + return null; + } + + /** + * Finds and returns {@link CalciteSchema} nested to the given rootSchema + * with specified schemaPath. + * + * <p>Uses the case-sensitivity policy of specified nameMatcher. + * + * <p>If not found, returns null. + * + * @param rootSchema root schema + * @param schemaPath full schema path of required schema + * @param nameMatcher name matcher + * + * @return CalciteSchema that corresponds specified schemaPath + */ + public static CalciteSchema getSchema(CalciteSchema rootSchema, + Iterable<String> schemaPath, SqlNameMatcher nameMatcher) { + CalciteSchema schema = rootSchema; + for (String schemaName : schemaPath) { + if (schema == rootSchema + && nameMatcher.matches(schemaName, schema.getName())) { + continue; + } + schema = schema.getSubSchema(schemaName, + nameMatcher.isCaseSensitive()); + if (schema == null) { + return null; + } + } + return schema; + } + + private static CalciteSchema.TableEntry getTableEntryFrom( + CalciteSchema schema, String name, boolean caseSensitive) { + CalciteSchema.TableEntry entry = + schema.getTable(name, caseSensitive); + if (entry == null) { + entry = schema.getTableBasedOnNullaryFunction(name, + caseSensitive); + } + return entry; + } + + /** * Returns whether there are any input columns that are sorted. * * <p>If so, it can be the default ORDER BY clause for a WINDOW specification. http://git-wip-us.apache.org/repos/asf/calcite/blob/26b4712d/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java b/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java index e0f0d94..e877b2a 100644 --- a/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java +++ b/core/src/test/java/org/apache/calcite/sql/test/SqlAdvisorTest.java @@ -68,6 +68,7 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { protected static final List<String> SALES_TABLES = Arrays.asList( "SCHEMA(CATALOG.SALES)", + "SCHEMA(CATALOG.SALES.NEST)", "TABLE(CATALOG.SALES.EMP)", "TABLE(CATALOG.SALES.EMPDEFAULTS)", "TABLE(CATALOG.SALES.EMPNULLABLES)", @@ -92,7 +93,8 @@ public class SqlAdvisorTest extends SqlValidatorTestCase { "SCHEMA(CATALOG.DYNAMIC)", "SCHEMA(CATALOG.SALES)", "SCHEMA(CATALOG.STRUCT)", - "SCHEMA(CATALOG.CUSTOMER)"); + "SCHEMA(CATALOG.CUSTOMER)", + "SCHEMA(CATALOG.SALES.NEST)"); private static final List<String> AB_TABLES = Arrays.asList( http://git-wip-us.apache.org/repos/asf/calcite/blob/26b4712d/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 5f96848..cd42747 100644 --- a/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java +++ b/core/src/test/java/org/apache/calcite/test/MockCatalogReader.java @@ -87,6 +87,7 @@ import org.apache.calcite.sql.validate.SqlMonotonicity; import org.apache.calcite.sql.validate.SqlNameMatcher; import org.apache.calcite.sql.validate.SqlNameMatchers; import org.apache.calcite.sql.validate.SqlValidatorCatalogReader; +import org.apache.calcite.sql.validate.SqlValidatorUtil; import org.apache.calcite.sql2rel.InitializerContext; import org.apache.calcite.sql2rel.InitializerExpressionFactory; import org.apache.calcite.sql2rel.NullInitializerExpressionFactory; @@ -527,6 +528,20 @@ public class MockCatalogReader extends CalciteCatalogReader { deptSlackingTable.addColumn("SLACKINGMIN", f.intType); registerTable(deptSlackingTable); + // Register nested schema NEST that contains table with a rolled up column. + MockSchema nestedSchema = new MockSchema("NEST"); + registerNestedSchema(schema, nestedSchema); + + // Register "EMP_R" table which contains a rolled up column in NEST schema. + ImmutableList<String> tablePath = + ImmutableList.of(schema.getCatalogName(), schema.name, nestedSchema.name, "EMP_R"); + final MockTable nestedEmpRolledTable = MockTable.create(this, tablePath, false, 14); + nestedEmpRolledTable.addColumn("EMPNO", f.intType, true); + nestedEmpRolledTable.addColumn("DEPTNO", f.intType); + nestedEmpRolledTable.addColumn("SLACKER", f.booleanType); + nestedEmpRolledTable.addColumn("SLACKINGMIN", f.intType); + nestedEmpRolledTable.registerRolledUpColumn("SLACKINGMIN"); + registerTable(nestedEmpRolledTable); } /** Adds some extra tables to the mock catalog. These increase the time and @@ -622,9 +637,10 @@ public class MockCatalogReader extends CalciteCatalogReader { private void registerTable(final List<String> names, final Table table) { assert names.get(0).equals(DEFAULT_CATALOG); - final String schemaName = names.get(1); - final String tableName = names.get(2); - final CalciteSchema schema = rootSchema.getSubSchema(schemaName, true); + final List<String> schemaPath = Util.skipLast(names); + final String tableName = Util.last(names); + final CalciteSchema schema = SqlValidatorUtil.getSchema(rootSchema, + schemaPath, SqlNameMatchers.withCaseSensitive(true)); schema.add(tableName, table); } @@ -632,6 +648,11 @@ public class MockCatalogReader extends CalciteCatalogReader { rootSchema.add(schema.name, new AbstractSchema()); } + private void registerNestedSchema(MockSchema parentSchema, MockSchema schema) { + rootSchema.getSubSchema(parentSchema.getName(), true) + .add(schema.name, new AbstractSchema()); + } + public RelDataType getNamedType(SqlIdentifier typeName) { if (typeName.equalsDeep(addressType.getSqlIdentifier(), Litmus.IGNORE)) { return addressType; @@ -829,6 +850,12 @@ public class MockCatalogReader extends CalciteCatalogReader { } public static MockTable create(MockCatalogReader catalogReader, + List<String> names, boolean stream, double rowCount) { + return new MockTable(catalogReader, names, stream, rowCount, null, + NullInitializerExpressionFactory.INSTANCE); + } + + public static MockTable create(MockCatalogReader catalogReader, MockSchema schema, String name, boolean stream, double rowCount, ColumnResolver resolver) { return create(catalogReader, schema, name, stream, rowCount, resolver, http://git-wip-us.apache.org/repos/asf/calcite/blob/26b4712d/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 f62b0aa..c2d89a5 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -10405,6 +10405,9 @@ public class SqlValidatorTest extends SqlValidatorTestCase { sql("select (((^slackingmin^))) from emp_r") .fails(error); + + sql("select ^slackingmin^ from nest.emp_r") + .fails(error); } @Test public void testSelectAggregateOnRolledUpColumn() {
