This is an automated email from the ASF dual-hosted git repository.
volodymyr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new 5da0d47 DRILL-7250: Query with CTE fails when its name matches to the
table name without access
5da0d47 is described below
commit 5da0d479a0fca9371487e4ea046fb75e253f47a5
Author: Volodymyr Vysotskyi <[email protected]>
AuthorDate: Wed May 15 00:45:24 2019 +0300
DRILL-7250: Query with CTE fails when its name matches to the table name
without access
---
.../org/apache/calcite/jdbc/DynamicRootSchema.java | 2 ++
.../drill/exec/planner/sql/SqlConverter.java | 41 ++++++++++------------
.../impersonation/TestImpersonationQueries.java | 16 ++++++++-
pom.xml | 2 +-
4 files changed, 36 insertions(+), 25 deletions(-)
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 10bde94..7de5d6a 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
@@ -61,6 +61,8 @@ public class DynamicRootSchema extends DynamicSchema {
@Override
protected CalciteSchema getImplicitSubSchema(String schemaName,
boolean caseSensitive) {
+ // Drill registers schemas in lower case, see AbstractSchema constructor
+ schemaName = schemaName == null ? null : schemaName.toLowerCase();
CalciteSchema retSchema = getSubSchemaMap().get(schemaName);
if (retSchema != null) {
return retSchema;
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
index 135ee26..e6de62e 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
@@ -26,6 +26,7 @@ import java.util.Objects;
import java.util.Properties;
import java.util.Set;
+import org.apache.calcite.jdbc.CalciteSchema;
import org.apache.calcite.util.Static;
import org.apache.commons.lang3.StringUtils;
import org.apache.drill.exec.planner.sql.parser.DrillParserUtil;
@@ -33,7 +34,6 @@ import
org.apache.drill.exec.planner.sql.parser.impl.DrillSqlParseException;
import org.apache.drill.common.expression.SchemaPath;
import org.apache.drill.exec.physical.base.MetadataProviderManager;
import org.apache.drill.exec.physical.base.TableMetadataProvider;
-import org.apache.drill.shaded.guava.com.google.common.base.Strings;
import org.apache.drill.shaded.guava.com.google.common.cache.CacheBuilder;
import org.apache.drill.shaded.guava.com.google.common.cache.CacheLoader;
import org.apache.drill.shaded.guava.com.google.common.cache.LoadingCache;
@@ -79,7 +79,6 @@ import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.sql2rel.SqlToRelConverter;
import org.apache.calcite.tools.RelBuilderFactory;
import org.apache.calcite.util.Util;
-import org.apache.commons.collections.ListUtils;
import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.common.types.Types;
@@ -271,7 +270,8 @@ public class SqlConverter {
RelDataType targetRowType,
SqlValidatorScope scope) {
if (node.getKind() == SqlKind.AS) {
- SqlNode sqlNode = ((SqlCall) node).operand(0);
+ SqlCall sqlCall = (SqlCall) node;
+ SqlNode sqlNode = sqlCall.operand(0);
switch (sqlNode.getKind()) {
case IDENTIFIER:
SqlIdentifier tempNode = (SqlIdentifier) sqlNode;
@@ -280,12 +280,10 @@ public class SqlConverter {
changeNamesIfTableIsTemporary(tempNode);
// Check the schema and throw a valid SchemaNotFound exception
instead of TableNotFound exception.
- if (catalogReader.getTable(tempNode.names) == null) {
- catalogReader.isValidSchema(tempNode.names);
- }
+ catalogReader.isValidSchema(tempNode.names);
break;
case UNNEST:
- if (((SqlCall) node).operandCount() < 3) {
+ if (sqlCall.operandCount() < 3) {
throw Static.RESOURCE.validationError("Alias table and column
name are required for UNNEST").ex();
}
}
@@ -670,7 +668,6 @@ public class SqlConverter {
private final DrillConfig drillConfig;
private final UserSession session;
private boolean allowTemporaryTables;
- private final SchemaPlus rootSchema;
private final LoadingCache<DrillTableKey, MetadataProviderManager>
tableCache;
@@ -685,7 +682,6 @@ public class SqlConverter {
this.drillConfig = drillConfig;
this.session = session;
this.allowTemporaryTables = true;
- this.rootSchema = rootSchema;
this.tableCache =
CacheBuilder.newBuilder()
.build(new CacheLoader<DrillTableKey, MetadataProviderManager>() {
@@ -759,27 +755,26 @@ public class SqlConverter {
}
/**
- * check if the schema provided is a valid schema:
+ * 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.
*/
- private void isValidSchema(final List<String> names) throws UserException {
- SchemaPlus defaultSchema = session.getDefaultSchema(this.rootSchema);
- String defaultSchemaCombinedPath =
SchemaUtilites.getSchemaPath(defaultSchema);
+ private void isValidSchema(List<String> names) throws UserException {
List<String> schemaPath = Util.skipLast(names);
- String schemaPathCombined = SchemaUtilites.getSchemaPath(schemaPath);
- String commonPrefix =
SchemaUtilites.getPrefixSchemaPath(defaultSchemaCombinedPath,
- schemaPathCombined,
- parserConfig.caseSensitive());
- boolean isPrefixDefaultPath = commonPrefix.length() ==
defaultSchemaCombinedPath.length();
- List<String> fullSchemaPath =
Strings.isNullOrEmpty(defaultSchemaCombinedPath) ? schemaPath :
- isPrefixDefaultPath ? schemaPath :
ListUtils.union(SchemaUtilites.getSchemaPathAsList(defaultSchema), schemaPath);
- if (names.size() > 1 && (SchemaUtilites.findSchema(this.rootSchema,
fullSchemaPath) == null &&
- SchemaUtilites.findSchema(this.rootSchema, schemaPath) == null))
{
- SchemaUtilites.throwSchemaNotFoundException(defaultSchema, schemaPath);
+
+ 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(defaultSchema, schemaPath);
}
/**
diff --git
a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationQueries.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationQueries.java
index 6cbd25a..718b6e9 100644
---
a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationQueries.java
+++
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationQueries.java
@@ -291,8 +291,22 @@ public class TestImpersonationQueries extends
BaseTestImpersonation {
}
}
+ @Test // DRILL-7250
+ public void testCTEWithImpersonation() throws Exception {
+ // Table lineitem is owned by "user0_1:group0_1" with permissions 750.
"user2_1" doesn't have access to it,
+ // but query uses CTE with the same name as the table, so query shouldn't
look for lineitem table
+ updateClient(org1Users[2]);
+ test("use %s", getWSSchema(org1Users[0]));
+ testBuilder()
+ .sqlQuery("with lineitem as (SELECT 1 as a) select * from lineitem")
+ .unOrdered()
+ .baselineColumns("a")
+ .baselineValues(1)
+ .go();
+ }
+
@AfterClass
- public static void removeMiniDfsBasedStorage() throws Exception {
+ public static void removeMiniDfsBasedStorage() {
getDrillbitContext().getStorage().deletePlugin(MINI_DFS_STORAGE_PLUGIN_NAME);
stopMiniDfsCluster();
}
diff --git a/pom.xml b/pom.xml
index f2b626d..a1ec48e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -50,7 +50,7 @@
<guava.version>19.0</guava.version>
<forkCount>2</forkCount>
<parquet.version>1.10.0</parquet.version>
- <calcite.version>1.18.0-drill-r1</calcite.version>
+ <calcite.version>1.18.0-drill-r2</calcite.version>
<avatica.version>1.13.0</avatica.version>
<janino.version>3.0.11</janino.version>
<sqlline.version>1.7.0</sqlline.version>