drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From volody...@apache.org
Subject [drill] branch master updated: DRILL-7250: Query with CTE fails when its name matches to the table name without access
Date Wed, 22 May 2019 09:42:09 GMT
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 <vvovyk@gmail.com>
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>


Mime
View raw message