DRILL-8380: Remove customised SqlValidatorImpl.deriveAlias (#2733)
diff --git a/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraQueryTest.java b/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraQueryTest.java
index 536a102..d99b1bf 100644
--- a/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraQueryTest.java
+++ b/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraQueryTest.java
@@ -337,7 +337,7 @@
fail("Query didn't fail");
} catch (UserRemoteException e) {
assertThat(e.getMessage(), containsString("VALIDATION ERROR"));
- assertThat(e.getMessage(), containsString("Schema [[cassandra, test_keyspace, non-existing]] is not valid with respect to either root schema or current default schema"));
+ assertThat(e.getMessage(), containsString("Object 'non-existing' not found within 'cassandra.test_keyspace'"));
}
}
diff --git a/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraUserTranslationTest.java b/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraUserTranslationTest.java
index f925fe3..82cf452 100644
--- a/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraUserTranslationTest.java
+++ b/contrib/storage-cassandra/src/test/java/org/apache/drill/exec/store/cassandra/CassandraUserTranslationTest.java
@@ -30,8 +30,9 @@
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.ADMIN_USER_PASSWORD;
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_1;
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_1_PASSWORD;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@Category({SlowTest.class})
@@ -97,7 +98,7 @@
client.queryBuilder().sql(sql).rowSet();
fail();
} catch (UserRemoteException e) {
- assertTrue(e.getMessage().contains("Schema [[ut_cassandra, test_keyspace]] is not valid"));
+ assertThat(e.getMessage(), containsString("Object 'ut_cassandra' not found"));
}
}
}
diff --git a/contrib/storage-elasticsearch/src/test/java/org/apache/drill/exec/store/elasticsearch/ElasticSearchQueryTest.java b/contrib/storage-elasticsearch/src/test/java/org/apache/drill/exec/store/elasticsearch/ElasticSearchQueryTest.java
index ebfcb13..9a0f64e 100644
--- a/contrib/storage-elasticsearch/src/test/java/org/apache/drill/exec/store/elasticsearch/ElasticSearchQueryTest.java
+++ b/contrib/storage-elasticsearch/src/test/java/org/apache/drill/exec/store/elasticsearch/ElasticSearchQueryTest.java
@@ -598,7 +598,7 @@
fail("Query didn't fail");
} catch (UserRemoteException e) {
assertThat(e.getMessage(), containsString("VALIDATION ERROR"));
- assertThat(e.getMessage(), containsString("Schema [[elastic, non-existing]] is not valid with respect to either root schema or current default schema"));
+ assertThat(e.getMessage(), containsString("Object 'non-existing' not found within 'elastic'"));
}
}
diff --git a/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcUserTranslation.java b/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcUserTranslation.java
index 2b34a28..a1086d9 100644
--- a/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcUserTranslation.java
+++ b/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcUserTranslation.java
@@ -49,6 +49,8 @@
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_1_PASSWORD;
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_2;
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_2_PASSWORD;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -163,7 +165,7 @@
client.queryBuilder().sql(sql).rowSet();
fail();
} catch (Exception e) {
- assertTrue(e.getMessage().contains("Schema [[mysql, drill_mysql_test]] is not valid"));
+ assertThat(e.getMessage(), containsString("Object 'mysql' not found"));
}
}
diff --git a/contrib/storage-splunk/src/test/java/org/apache/drill/exec/store/splunk/TestSplunkUserTranslation.java b/contrib/storage-splunk/src/test/java/org/apache/drill/exec/store/splunk/TestSplunkUserTranslation.java
index 63de266..d115b1e 100644
--- a/contrib/storage-splunk/src/test/java/org/apache/drill/exec/store/splunk/TestSplunkUserTranslation.java
+++ b/contrib/storage-splunk/src/test/java/org/apache/drill/exec/store/splunk/TestSplunkUserTranslation.java
@@ -31,8 +31,9 @@
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.ADMIN_USER_PASSWORD;
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_1;
import static org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl.TEST_USER_1_PASSWORD;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@Category({SlowTest.class})
@@ -97,7 +98,7 @@
client.queryBuilder().sql(sql).rowSet();
fail();
} catch (UserRemoteException e) {
- assertTrue(e.getMessage().contains("Schema [[ut_splunk]] is not valid"));
+ assertThat(e.getMessage(), containsString("Object 'ut_splunk' not found"));
}
}
}
diff --git a/exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java b/exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
index 380f0d5..71925a2 100644
--- a/exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
+++ b/exec/java-exec/src/main/java/org/apache/calcite/jdbc/DynamicRootSchema.java
@@ -18,13 +18,13 @@
package org.apache.calcite.jdbc;
import org.apache.calcite.DataContext;
-
import org.apache.calcite.linq4j.tree.Expression;
import org.apache.calcite.linq4j.tree.Expressions;
import org.apache.calcite.schema.SchemaPlus;
import org.apache.calcite.util.BuiltInMethod;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.exceptions.UserExceptionUtils;
+import org.apache.drill.common.expression.PathSegment;
import org.apache.drill.common.expression.SchemaPath;
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.alias.AliasRegistryProvider;
@@ -89,7 +89,7 @@
return retSchema;
}
- public SchemaPath resolveTableAlias(String alias) {
+ private SchemaPath resolveTableAlias(String alias) {
return Optional.ofNullable(aliasRegistryProvider.getTableAliasesRegistry()
.getUserAliases(schemaConfig.getUserName()).get(alias))
.map(SchemaPath::parseFromString)
@@ -218,9 +218,39 @@
}
}
+ @Override
+ protected TableEntry getImplicitTable(String tableName, boolean caseSensitive) {
+ return Optional.ofNullable(getTemporaryTable(tableName, caseSensitive))
+ .<TableEntry>map(table -> new TableEntryImpl(this, tableName, table.getTable(), table.sqls))
+ .orElse(super.getImplicitTable(tableName, true));
+ }
+
+ private TableEntry getTemporaryTable(String tableName, boolean caseSensitive) {
+ CalciteSchema currentSchema = this;
+
+ PathSegment.NameSegment pathSegment =
+ Optional.ofNullable(resolveTableAlias(SchemaPath.getCompoundPath(tableName).toExpr()))
+ .map(SchemaPath::getRootSegment)
+ .orElse(null);
+
+ if (pathSegment == null) {
+ return null;
+ }
+
+ while (!pathSegment.isLastPath()) {
+ currentSchema = currentSchema.getImplicitSubSchema(pathSegment.getPath(), caseSensitive);
+ pathSegment = pathSegment.getChild().getNameSegment();
+ }
+
+ if (currentSchema != null) {
+ return currentSchema.getTable(pathSegment.getNameSegment().getPath(), caseSensitive);
+ }
+ return null;
+ }
+
public static class RootSchema extends AbstractSchema {
- private StoragePluginRegistry storages;
+ private final StoragePluginRegistry storages;
public RootSchema(StoragePluginRegistry storages) {
super(Collections.emptyList(), ROOT_SCHEMA_NAME);
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContextImpl.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContextImpl.java
index dafbf88..87dbcb4 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContextImpl.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContextImpl.java
@@ -727,5 +727,15 @@
public OptionValue getOption(String optionKey) {
return optionManager.getOption(optionKey);
}
+
+ @Override
+ public String getTemporaryTableName(String table) {
+ throw new UnsupportedOperationException("getTemporaryTableName is not supported");
+ }
+
+ @Override
+ public String getTemporaryWorkspace() {
+ throw new UnsupportedOperationException("getTemporaryWorkspace is not supported");
+ }
}
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
index 3aef43c..9ac1df0 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java
@@ -274,6 +274,16 @@
return getOptions().getOption(optionKey);
}
+ @Override
+ public String getTemporaryTableName(String table) {
+ return session.resolveTemporaryTableName(table);
+ }
+
+ @Override
+ public String getTemporaryWorkspace() {
+ return getConfig().getString(ExecConstants.DEFAULT_TEMPORARY_WORKSPACE);
+ }
+
public boolean isImpersonationEnabled() {
return getConfig().getBoolean(ExecConstants.IMPERSONATION_ENABLED);
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
index b6fdcc5..244e8dc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java
@@ -17,25 +17,20 @@
*/
package org.apache.drill.exec.planner.sql.conversion;
-import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import java.util.function.BooleanSupplier;
-import java.util.function.Supplier;
import org.apache.calcite.adapter.java.JavaTypeFactory;
import org.apache.calcite.config.CalciteConnectionConfigImpl;
import org.apache.calcite.config.CalciteConnectionProperty;
-import org.apache.calcite.jdbc.CalciteSchema;
import org.apache.calcite.jdbc.DynamicSchema;
import org.apache.calcite.prepare.CalciteCatalogReader;
import org.apache.calcite.prepare.Prepare;
import org.apache.calcite.schema.SchemaPlus;
-import org.apache.calcite.sql.validate.SqlValidatorUtil;
-import org.apache.calcite.util.Util;
+import org.apache.calcite.sql.validate.SqlNameMatchers;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.UserException;
-import org.apache.drill.common.util.DrillStringUtils;
import org.apache.drill.exec.metastore.MetadataProviderManager;
import org.apache.drill.exec.planner.logical.DrillTable;
import org.apache.drill.exec.planner.sql.SchemaUtilites;
@@ -58,10 +53,8 @@
private final DrillConfig drillConfig;
private final UserSession session;
- private final String temporarySchema;
private boolean allowTemporaryTables;
private final BooleanSupplier useRootSchema;
- private final Supplier<SchemaPlus> defaultSchemaSupplier;
private final LoadingCache<DrillTableKey, MetadataProviderManager> tableCache;
@@ -71,11 +64,10 @@
JavaTypeFactory typeFactory,
DrillConfig drillConfig,
UserSession session,
- String temporarySchema,
- BooleanSupplier useRootSchema,
- Supplier<SchemaPlus> defaultSchemaSupplier) {
- super(DynamicSchema.from(rootSchema), defaultSchema,
- typeFactory, getConnectionConfig(caseSensitive));
+ BooleanSupplier useRootSchema) {
+ super(DynamicSchema.from(rootSchema), SqlNameMatchers.withCaseSensitive(caseSensitive),
+ ImmutableList.of(defaultSchema, ImmutableList.of()),
+ typeFactory, getConnectionConfig(caseSensitive));
this.drillConfig = drillConfig;
this.session = session;
this.allowTemporaryTables = true;
@@ -87,9 +79,7 @@
return key.getMetadataProviderManager();
}
});
- this.temporarySchema = temporarySchema;
this.useRootSchema = useRootSchema;
- this.defaultSchemaSupplier = defaultSchemaSupplier;
}
/**
@@ -99,19 +89,6 @@
this.allowTemporaryTables = false;
}
- List<String> getTemporaryNames(List<String> names) {
- if (needsTemporaryTableCheck(names, session.getDefaultSchemaPath(), drillConfig)) {
- String tableName = DrillStringUtils.removeLeadingSlash(names.get(names.size() - 1));
- String temporaryTableName = session.resolveTemporaryTableName(tableName);
- if (temporaryTableName != null) {
- List<String> temporaryNames = new ArrayList<>(SchemaUtilites.getSchemaPathAsList(temporarySchema));
- temporaryNames.add(temporaryTableName);
- return temporaryNames;
- }
- }
- return null;
- }
-
/**
* If schema is not indicated (only one element in the list) or schema is default temporary workspace,
* we need to check among session temporary tables in default temporary workspace first.
@@ -135,14 +112,15 @@
}
private void checkTemporaryTable(List<String> names) {
- if (allowTemporaryTables) {
+ if (allowTemporaryTables || !needsTemporaryTableCheck(names, session.getDefaultSchemaPath(), drillConfig)) {
return;
}
- String originalTableName = session.getOriginalTableNameFromTemporaryTable(names.get(names.size() - 1));
+ String tableName = names.get(names.size() - 1);
+ String originalTableName = session.resolveTemporaryTableName(tableName);
if (originalTableName != null) {
throw UserException
.validationError()
- .message("Temporary tables usage is disallowed. Used temporary table name: [%s].", originalTableName)
+ .message("A reference to temporary table [%s] was made in a context where temporary table references are not allowed.", tableName)
.build(logger);
}
}
@@ -156,29 +134,6 @@
}
/**
- * Checks if the schema provided is a valid schema:
- * <li>schema is not indicated (only one element in the names list)<li/>
- *
- * @param names list of schema and table names, table name is always the last element
- * @throws UserException if the schema is not valid.
- */
- void isValidSchema(List<String> names) throws UserException {
- List<String> schemaPath = Util.skipLast(names);
-
- for (List<String> currentSchema : getSchemaPaths()) {
- List<String> fullSchemaPath = new ArrayList<>(currentSchema);
- fullSchemaPath.addAll(schemaPath);
- CalciteSchema schema = SqlValidatorUtil.getSchema(getRootSchema(),
- fullSchemaPath, nameMatcher());
-
- if (schema != null) {
- return;
- }
- }
- SchemaUtilites.throwSchemaNotFoundException(defaultSchemaSupplier.get(), schemaPath);
- }
-
- /**
* We should check if passed table is temporary or not if:
* <li>schema is not indicated (only one element in the names list)<li/>
* <li>current schema or indicated schema is default temporary workspace<li/>
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillValidator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillValidator.java
deleted file mode 100644
index 5de8a34..0000000
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillValidator.java
+++ /dev/null
@@ -1,140 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.drill.exec.planner.sql.conversion;
-
-import org.apache.calcite.jdbc.CalciteSchema;
-import org.apache.calcite.jdbc.DynamicRootSchema;
-import org.apache.calcite.rel.type.RelDataType;
-import org.apache.calcite.rel.type.RelDataTypeFactory;
-import org.apache.calcite.sql.SqlCall;
-import org.apache.calcite.sql.SqlIdentifier;
-import org.apache.calcite.sql.SqlKind;
-import org.apache.calcite.sql.SqlNode;
-import org.apache.calcite.sql.SqlOperatorTable;
-import org.apache.calcite.sql.parser.SqlParserPos;
-import org.apache.calcite.sql.validate.SqlConformance;
-import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
-import org.apache.calcite.sql.validate.SqlValidatorImpl;
-import org.apache.calcite.sql.validate.SqlValidatorScope;
-import org.apache.calcite.sql.validate.SqlValidatorUtil;
-import org.apache.calcite.util.Static;
-import org.apache.drill.common.expression.PathSegment;
-import org.apache.drill.common.expression.SchemaPath;
-import org.apache.drill.exec.util.ImpersonationUtil;
-import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
-
-import java.security.PrivilegedAction;
-import java.util.ArrayList;
-import java.util.List;
-
-class DrillValidator extends SqlValidatorImpl {
-
- private final boolean isImpersonationEnabled;
-
- DrillValidator(SqlOperatorTable opTab, SqlValidatorCatalogReader catalogReader,
- RelDataTypeFactory typeFactory, SqlConformance conformance, boolean isImpersonationEnabled) {
- super(opTab, catalogReader, typeFactory,
- Config.DEFAULT.withConformance(conformance)
- .withTypeCoercionEnabled(true)
- .withIdentifierExpansion(true));
- this.isImpersonationEnabled = isImpersonationEnabled;
- }
-
- @Override
- protected void validateFrom(SqlNode node, RelDataType targetRowType, SqlValidatorScope scope) {
- if (node.getKind() == SqlKind.AS) {
- SqlCall sqlCall = (SqlCall) node;
- SqlNode sqlNode = sqlCall.operand(0);
- switch (sqlNode.getKind()) {
- case IDENTIFIER:
- SqlIdentifier tempNode = (SqlIdentifier) sqlNode;
- changeNamesIfTableIsTemporary(tempNode);
- replaceAliasWithActualName(tempNode);
- // Check the schema and throw a valid SchemaNotFound exception instead of TableNotFound exception.
- ((DrillCalciteCatalogReader) getCatalogReader()).isValidSchema(tempNode.names);
- break;
- case UNNEST:
- if (sqlCall.operandCount() < 3) {
- throw Static.RESOURCE.validationError("Alias table and column name are required for UNNEST").ex();
- }
- }
- }
- if (isImpersonationEnabled) {
- ImpersonationUtil.getProcessUserUGI().doAs((PrivilegedAction<Void>) () -> {
- super.validateFrom(node, targetRowType, scope);
- return null;
- });
- } else {
- super.validateFrom(node, targetRowType, scope);
- }
- }
-
- private void replaceAliasWithActualName(SqlIdentifier tempNode) {
- CalciteSchema schema = getCatalogReader().getRootSchema();
- if (schema instanceof DynamicRootSchema) {
- DynamicRootSchema rootSchema = (DynamicRootSchema) schema;
- String alias = SchemaPath.getCompoundPath(tempNode.names.toArray(new String[0])).toExpr();
- SchemaPath actualPath = rootSchema.resolveTableAlias(alias);
- if (actualPath != null) {
- List<String> names = new ArrayList<>();
- PathSegment pathSegment = actualPath.getRootSegment();
- while (pathSegment != null) {
- names.add(pathSegment.getNameSegment().getPath());
- pathSegment = pathSegment.getChild();
- }
- changeNames(tempNode, names);
- }
- }
- }
-
- @Override
- public String deriveAlias(SqlNode node, int ordinal) {
- if (node instanceof SqlIdentifier) {
- SqlIdentifier sqlIdentifier = (SqlIdentifier) node;
- changeNamesIfTableIsTemporary(sqlIdentifier);
- replaceAliasWithActualName(sqlIdentifier);
- }
- return SqlValidatorUtil.getAlias(node, ordinal);
- }
-
- @Override
- protected void inferUnknownTypes(RelDataType inferredType, SqlValidatorScope scope, SqlNode node) {
- // calls validateQuery() for SqlSelect to be sure that temporary table name will be changed
- // for the case when it is used in sub-select
- if (node.getKind() == SqlKind.SELECT) {
- validateQuery(node, scope, inferredType);
- }
- super.inferUnknownTypes(inferredType, scope, node);
- }
-
- private void changeNamesIfTableIsTemporary(SqlIdentifier tempNode) {
- List<String> temporaryTableNames = ((DrillCalciteCatalogReader) getCatalogReader()).getTemporaryNames(tempNode.names);
- if (temporaryTableNames != null) {
- changeNames(tempNode, temporaryTableNames);
- }
- }
-
- private void changeNames(SqlIdentifier sqlIdentifier, List<String> newNames) {
- SqlParserPos pos = sqlIdentifier.getComponentParserPosition(0);
- List<SqlParserPos> poses = Lists.newArrayList();
- for (int i = 0; i < newNames.size(); i++) {
- poses.add(i, pos);
- }
- sqlIdentifier.setNames(newNames, poses);
- }
-}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillViewExpander.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillViewExpander.java
index 6e9e0da..2f4f3fc 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillViewExpander.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillViewExpander.java
@@ -68,9 +68,8 @@
sqlConverter.getTypeFactory(),
sqlConverter.getDrillConfig(),
sqlConverter.getSession(),
- sqlConverter.getTemporarySchema(),
- sqlConverter::useRootSchema,
- sqlConverter::getDefaultSchema);
+ sqlConverter::useRootSchema
+ );
}
private SchemaPlus findSchema(String queryString, SchemaPlus rootSchema, List<String> schemaPath) {
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java
index 76452bb..303db43 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java
@@ -17,11 +17,6 @@
*/
package org.apache.drill.exec.planner.sql.conversion;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-import java.util.stream.Collectors;
-
import org.apache.calcite.adapter.java.JavaTypeFactory;
import org.apache.calcite.avatica.util.Casing;
import org.apache.calcite.jdbc.DynamicSchema;
@@ -46,6 +41,8 @@
import org.apache.calcite.sql.parser.SqlParser;
import org.apache.calcite.sql.util.ChainedSqlOperatorTable;
import org.apache.calcite.sql.validate.SqlConformance;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.sql2rel.SqlToRelConverter;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.UserException;
@@ -65,10 +62,17 @@
import org.apache.drill.exec.planner.sql.parser.impl.DrillSqlParseException;
import org.apache.drill.exec.planner.types.DrillRelDataTypeSystem;
import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.util.ImpersonationUtil;
import org.apache.drill.exec.util.Utilities;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.security.PrivilegedAction;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
/**
* Class responsible for managing:
* <ul>
@@ -89,7 +93,7 @@
private final SchemaPlus defaultSchema;
private final SqlOperatorTable opTab;
private final RelOptCostFactory costFactory;
- private final DrillValidator validator;
+ private final SqlValidator validator;
private final boolean isInnerQuery;
private final boolean isExpandedView;
private final QueryContext util;
@@ -102,6 +106,7 @@
private RelOptCluster cluster;
private VolcanoPlanner planner;
private boolean useRootSchema = false;
+ private final boolean isImpersonationEnabled;
public SqlConverter(QueryContext context) {
this.settings = context.getPlannerSettings();
@@ -143,13 +148,15 @@
typeFactory,
drillConfig,
session,
- temporarySchema,
- this::useRootSchema,
- this::getDefaultSchema);
+ this::useRootSchema
+ );
this.opTab = new ChainedSqlOperatorTable(Arrays.asList(context.getDrillOperatorTable(), catalog));
this.costFactory = (settings.useDefaultCosting()) ? null : new DrillCostBase.DrillCostFactory();
- this.validator =
- new DrillValidator(opTab, catalog, typeFactory, parserConfig.conformance(), context.isImpersonationEnabled());
+ this.validator = SqlValidatorUtil.newValidator(opTab, catalog, typeFactory,
+ SqlValidator.Config.DEFAULT.withConformance(parserConfig.conformance())
+ .withTypeCoercionEnabled(true)
+ .withIdentifierExpansion(true));
+ this.isImpersonationEnabled = context.isImpersonationEnabled();
cluster = null;
}
@@ -169,12 +176,15 @@
this.catalog = catalog;
this.opTab = parent.opTab;
this.planner = parent.planner;
- this.validator =
- new DrillValidator(opTab, catalog, typeFactory, parserConfig.conformance(), util.isImpersonationEnabled());
+ this.validator = SqlValidatorUtil.newValidator(opTab, catalog, typeFactory,
+ SqlValidator.Config.DEFAULT.withConformance(parserConfig.conformance())
+ .withTypeCoercionEnabled(true)
+ .withIdentifierExpansion(true));
this.temporarySchema = parent.temporarySchema;
this.session = parent.session;
this.drillConfig = parent.drillConfig;
this.cluster = parent.cluster;
+ this.isImpersonationEnabled = util.isImpersonationEnabled();
}
public SqlNode parse(String sql) {
@@ -195,7 +205,12 @@
public SqlNode validate(final SqlNode parsedNode) {
try {
- return validator.validate(parsedNode);
+ if (isImpersonationEnabled) {
+ return ImpersonationUtil.getProcessUserUGI().doAs(
+ (PrivilegedAction<SqlNode>) () -> validator.validate(parsedNode));
+ } else {
+ return validator.validate(parsedNode);
+ }
} catch (RuntimeException e) {
UserException.Builder builder = UserException
.validationError(e);
@@ -237,7 +252,7 @@
return validator.getValidatedNodeType(validatedNode);
}
- public DrillValidator getValidator() {
+ public SqlValidator getValidator() {
return validator;
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaConfig.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaConfig.java
index 19906fa..71b9304 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaConfig.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaConfig.java
@@ -40,6 +40,14 @@
this.ignoreAuthErrors = ignoreAuthErrors;
}
+ public String getTemporaryTableName(String table) {
+ return provider.getTemporaryTableName(table);
+ }
+
+ public String getTemporaryWorkspace() {
+ return provider.getTemporaryWorkspace();
+ }
+
/**
* Create new builder.
* @param userName Name of the user accessing the storage sources.
@@ -113,5 +121,9 @@
UserCredentials getQueryUserCredentials();
OptionValue getOption(String optionKey);
+
+ String getTemporaryTableName(String table);
+
+ String getTemporaryWorkspace();
}
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
index 3ab2643..5c698ec 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
@@ -71,6 +71,16 @@
return options.getOption(optionKey);
}
+ @Override
+ public String getTemporaryTableName(String table) {
+ throw new UnsupportedOperationException("getTemporaryTableName is not supported");
+ }
+
+ @Override
+ public String getTemporaryWorkspace() {
+ throw new UnsupportedOperationException("getTemporaryWorkspace is not supported");
+ }
+
@Override public SchemaPlus getRootSchema(String userName) {
return createRootSchema(userName, this);
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
index b117b43..7023d81 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java
@@ -63,6 +63,7 @@
import org.apache.drill.exec.planner.logical.DynamicDrillTable;
import org.apache.drill.exec.planner.logical.FileSystemCreateTableEntry;
import org.apache.drill.exec.planner.sql.ExpandingConcurrentMap;
+import org.apache.drill.exec.planner.sql.SchemaUtilites;
import org.apache.drill.exec.record.metadata.schema.FsMetastoreSchemaProvider;
import org.apache.drill.exec.store.AbstractSchema;
import org.apache.drill.exec.store.PartitionNotFoundException;
@@ -403,8 +404,24 @@
return f.getView(mapper);
}
+ private String getTemporaryName(String name) {
+ if (isTemporaryWorkspace()) {
+ String tableName = DrillStringUtils.removeLeadingSlash(name);
+ return schemaConfig.getTemporaryTableName(tableName);
+ }
+ return null;
+ }
+
+ private boolean isTemporaryWorkspace() {
+ return SchemaUtilites.getSchemaPath(schemaPath).equals(schemaConfig.getTemporaryWorkspace());
+ }
+
@Override
public Table getTable(String tableName) {
+ String temporaryName = getTemporaryName(tableName);
+ if (temporaryName != null) {
+ tableName = temporaryName;
+ }
TableInstance tableKey = new TableInstance(TableSignature.of(tableName), ImmutableList.of());
// first check existing tables.
if (tables.alreadyContainsKey(tableKey)) {
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/MetadataProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/MetadataProvider.java
index 2d2dd90..f9c0195 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/MetadataProvider.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/MetadataProvider.java
@@ -37,6 +37,7 @@
import org.apache.drill.common.FunctionNames;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.ErrorHelper;
+import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.ops.ViewExpansionContext;
import org.apache.drill.exec.proto.UserBitShared.DrillPBError;
import org.apache.drill.exec.proto.UserBitShared.DrillPBError.ErrorType;
@@ -607,6 +608,16 @@
@Override public UserCredentials getQueryUserCredentials() {
return session.getCredentials();
}
+
+ @Override
+ public String getTemporaryTableName(String table) {
+ return session.resolveTemporaryTableName(table);
+ }
+
+ @Override
+ public String getTemporaryWorkspace() {
+ return config.getString(ExecConstants.DEFAULT_TEMPORARY_WORKSPACE);
+ }
};
}
diff --git a/exec/java-exec/src/test/java/org/apache/drill/alias/TestAliasSubstitution.java b/exec/java-exec/src/test/java/org/apache/drill/alias/TestAliasSubstitution.java
index 6266c8c..c84d490 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/alias/TestAliasSubstitution.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/alias/TestAliasSubstitution.java
@@ -157,7 +157,7 @@
fail();
} catch (UserRemoteException e) {
MatcherAssert.assertThat(e.getVerboseMessage(),
- containsString("VALIDATION ERROR: Schema [[foobar]] is not valid with respect to either root schema or current default schema"));
+ containsString("Object 'foobar' not found: Object 'foobar' not found"));
} finally {
storageAliasesRegistry.deletePublicAliases();
client.resetSystem(ExecConstants.ENABLE_ALIASES);
@@ -219,7 +219,7 @@
fail();
} catch (UserRemoteException e) {
MatcherAssert.assertThat(e.getVerboseMessage(),
- containsString("VALIDATION ERROR: Schema [[foobar]] is not valid with respect to either root schema or current default schema."));
+ containsString("Object 'foobar' not found: Object 'foobar' not found"));
} finally {
storageAliasesRegistry.deleteUserAliases(TEST_USER_2);
}
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java
index b716aab..05d7df5 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/lateraljoin/TestLateralPlans.java
@@ -303,7 +303,7 @@
.sql(sql)
.run();
} catch (UserRemoteException ex) {
- assertTrue(ex.getMessage().contains("Alias table and column name are required for UNNEST"));
+ assertThat(ex.getMessage(), containsString("Alias table and column name are required for UNNEST"));
}
}
@@ -320,7 +320,7 @@
.sql(sql)
.run();
} catch (UserRemoteException ex) {
- assertTrue(ex.getMessage().contains("Alias table and column name are required for UNNEST"));
+ assertThat(ex.getMessage(), containsString("Column 'orders' not found in table 't2'"));
}
}
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
index 187e794..41ed71e 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTTAS.java
@@ -89,17 +89,11 @@
test("create TEMPORARY table %s as select 'A' as c1 from (values(1))", temporaryTableName);
testBuilder()
- .sqlQuery("select * from %s", temporaryTableName)
+ .sqlQuery("select * from %s.%s", DFS_TMP_SCHEMA, temporaryTableName)
.unOrdered()
.baselineColumns("c1")
.baselineValues("A")
.go();
-
- testBuilder()
- .sqlQuery("select * from %s", temporaryTableName)
- .unOrdered()
- .sqlBaselineQuery("select * from %s.%s", DFS_TMP_SCHEMA, temporaryTableName)
- .go();
}
} finally {
resetSessionOption("store.format");
@@ -117,7 +111,7 @@
test("create TEMPORARY table %s as select 'A' as c1 from (values(1))", temporaryTableName);
for (String tableName : temporaryTableNames) {
testBuilder()
- .sqlQuery("select * from %s", tableName)
+ .sqlQuery("select * from %s.%s", DFS_TMP_SCHEMA, tableName)
.unOrdered()
.baselineColumns("c1")
.baselineValues("A")
@@ -237,7 +231,8 @@
test("create TEMPORARY table %s as select 'A' as c1, 'C' as c2 from (values(1))", temporaryRightTableName);
testBuilder()
- .sqlQuery("select t1.c2 col1, t2.c2 col2 from %s t1 join %s t2 on t1.c1 = t2.c1", temporaryLeftTableName, temporaryRightTableName)
+ .sqlQuery("select t1.c2 col1, t2.c2 col2 from %s.%s t1 join %s.%s t2 on t1.c1 = t2.c1",
+ DFS_TMP_SCHEMA, temporaryLeftTableName, DFS_TMP_SCHEMA, temporaryRightTableName)
.unOrdered()
.baselineColumns("col1", "col2")
.baselineValues("B", "C")
@@ -252,13 +247,6 @@
test("create table %s as select 'persistent_table' as c1 from (values(1))", name);
testBuilder()
- .sqlQuery("select * from %s", name)
- .unOrdered()
- .baselineColumns("c1")
- .baselineValues("temporary_table")
- .go();
-
- testBuilder()
.sqlQuery("select * from %s.%s", temp2_schema, name)
.unOrdered()
.baselineColumns("c1")
@@ -283,7 +271,7 @@
test("create view %s as select 'view' as c1 from (values(1))", name);
testBuilder()
- .sqlQuery("select * from %s", name)
+ .sqlQuery("select * from %s.%s", DFS_TMP_SCHEMA, name)
.unOrdered()
.baselineColumns("c1")
.baselineValues("temporary_table")
@@ -312,9 +300,9 @@
test("create TEMPORARY table %s as select 'A' as c1 from (values(1))", temporaryTableName);
expectUserRemoteExceptionWithMessage(String.format(
- "VALIDATION ERROR: Temporary tables usage is disallowed. Used temporary table name: [%s]", temporaryTableName));
+ "VALIDATION ERROR: A reference to temporary table [%s] was made in a context where temporary table references are not allowed.", temporaryTableName));
- test("create view %s.view_with_temp_table as select * from %s", DFS_TMP_SCHEMA, temporaryTableName);
+ test("create view %s.view_with_temp_table as select * from %s.%s", DFS_TMP_SCHEMA, DFS_TMP_SCHEMA, temporaryTableName);
}
@Test
@@ -336,7 +324,7 @@
test("create temporary table %s as select 'TEMP' as c1 from (values(1))", tableName);
expectUserRemoteExceptionWithMessage(String.format(
- "VALIDATION ERROR: Temporary tables usage is disallowed. Used temporary table name: [%s]", tableName));
+ "VALIDATION ERROR: A reference to temporary table [%s] was made in a context where temporary table references are not allowed.", tableName));
test("select * from %s", viewName);
}
@@ -424,7 +412,7 @@
test("create TEMPORARY table %s as select 'A' as c1 from (values(1))", temporaryTableName);
expectUserRemoteExceptionWithMessage(String.format(
- "VALIDATION ERROR: Unknown view [%s] in schema [%s]", temporaryTableName, DFS_TMP_SCHEMA));
+ "VALIDATION ERROR: [%s] is not a VIEW in schema [%s]", temporaryTableName, DFS_TMP_SCHEMA));
test("drop view %s.%s", DFS_TMP_SCHEMA, temporaryTableName);
}
@@ -433,8 +421,8 @@
public void testJoinTemporaryWithPersistentTable() throws Exception {
String temporaryTableName = "temp_tab";
String persistentTableName = "pers_tab";
- String query = String.format("select * from `%s` a join `%s` b on a.c1 = b.c2",
- persistentTableName, temporaryTableName);
+ String query = String.format("select * from `%s` a join %s.`%s` b on a.c1 = b.c2",
+ persistentTableName, DFS_TMP_SCHEMA, temporaryTableName);
test("use %s", temp2_schema);
test("create TEMPORARY table %s as select '12312' as c2", temporaryTableName);
@@ -472,17 +460,17 @@
try {
test("CREATE TEMPORARY TABLE %s AS SELECT * FROM cp.`region.json`", tableName);
- String query = "SELECT region_id FROM `%s` LIMIT 1";
+ String query = "SELECT region_id FROM %s.`%s` LIMIT 1";
testBuilder()
- .sqlQuery(query, tableName)
+ .sqlQuery(query, DFS_TMP_SCHEMA, tableName)
.unOrdered()
.baselineColumns("region_id")
.baselineValues(0L)
.go();
testBuilder()
- .sqlQuery(query, "/" + tableName)
+ .sqlQuery(query, DFS_TMP_SCHEMA, "/" + tableName)
.unOrdered()
.baselineColumns("region_id")
.baselineValues(0L)
@@ -516,11 +504,11 @@
String query =
"select t1.id as id,\n" +
"(select count(t2.id)\n" +
- "from source t2 where t2.id = t1.id) as c\n" +
- "from source t1";
+ "from %s.source t2 where t2.id = t1.id) as c\n" +
+ "from %s.source t1";
testBuilder()
- .sqlQuery(query)
+ .sqlQuery(query, DFS_TMP_SCHEMA, DFS_TMP_SCHEMA)
.ordered()
.baselineColumns("id", "c")
.baselineValues(1, 1L)
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestSchemaNotFoundException.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestSchemaNotFoundException.java
index dcd25bc..c73d9ea 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestSchemaNotFoundException.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestSchemaNotFoundException.java
@@ -21,6 +21,8 @@
import org.apache.drill.test.TestTools;
import org.junit.Test;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertTrue;
public class TestSchemaNotFoundException extends BaseTestQuery {
@@ -32,10 +34,8 @@
try {
testNoResult(query);
} catch (Exception ex) {
- final String pattern = String.format("[[dfs1]] is not valid with respect to either root schema or current default schema").toLowerCase();
- final boolean isSchemaNotFound = ex.getMessage().toLowerCase().contains(pattern);
- assertTrue(isSchemaNotFound);
- throw ex;
+ assertThat(ex.getMessage(), containsString("Object 'dfs1' not found"));
+ throw ex;
}
}
@@ -46,10 +46,8 @@
try {
testNoResult(query);
} catch (Exception ex) {
- final String pattern = String.format("[[dfs, tmp1]] is not valid with respect to either root schema or current default schema").toLowerCase();
- final boolean isSchemaNotFound = ex.getMessage().toLowerCase().contains(pattern);
- assertTrue(isSchemaNotFound);
- throw ex;
+ assertThat(ex.getMessage(), containsString("Object 'tmp1' not found within 'dfs'"));
+ throw ex;
}
}
@@ -61,10 +59,8 @@
testNoResult("use dfs");
testNoResult(query);
} catch (Exception ex) {
- final String pattern = String.format("[[tmp1]] is not valid with respect to either root schema or current default schema").toLowerCase();
- final boolean isSchemaNotFound = ex.getMessage().toLowerCase().contains(pattern);
- assertTrue(isSchemaNotFound);
- throw ex;
+ assertThat(ex.getMessage(), containsString("Object 'tmp1' not found"));
+ throw ex;
}
}
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/store/TestDisabledPlugin.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/store/TestDisabledPlugin.java
index 74add91..805e126 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/store/TestDisabledPlugin.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/store/TestDisabledPlugin.java
@@ -30,9 +30,10 @@
import org.junit.experimental.categories.Category;
import static org.apache.drill.exec.util.StoragePluginTestUtils.CP_PLUGIN_NAME;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@Category(SqlTest.class)
@@ -62,8 +63,8 @@
fail("Query should have failed!");
} catch (UserRemoteException e) {
assertEquals(UserBitShared.DrillPBError.ErrorType.VALIDATION, e.getErrorType());
- assertTrue("Incorrect error message",
- e.getMessage().contains("VALIDATION ERROR: Schema"));
+ assertThat("Incorrect error message", e.getMessage(),
+ containsString("VALIDATION ERROR: From line 1, column 15 to line 1, column 32: Object 'cp' not found"));
}
}
@@ -74,8 +75,8 @@
fail("Query should have failed!");
} catch (UserRemoteException e) {
assertEquals(UserBitShared.DrillPBError.ErrorType.VALIDATION, e.getErrorType());
- assertTrue("Incorrect error message",
- e.getMessage().contains("VALIDATION ERROR: Schema"));
+ assertThat("Incorrect error message", e.getMessage(),
+ containsString("VALIDATION ERROR: Schema"));
}
}
