Uploaded image for project: 'Apache Drill'
  1. Apache Drill
  2. DRILL-5238

CTTAS: unable to resolve temporary table if workspace is indicated without schema

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.10.0
    • 1.10.0
    • None
    • None

    Description

      Drill is unable to resolve temporary table if default temporary workspace is partially indicated (schema was set using USE command and only workspace name is used in query).

      0: jdbc:drill:zk=local> use dfs;
      +-------+----------------------------------+
      |  ok   |             summary              |
      +-------+----------------------------------+
      | true  | Default schema changed to [dfs]  |
      +-------+----------------------------------+
      1 row selected (0.156 seconds)
      
      0: jdbc:drill:zk=local> create temporary table tmp.t as select 'A' from (values(1));
      +-----------+----------------------------+
      | Fragment  | Number of records written  |
      +-----------+----------------------------+
      | 0_0       | 1                          |
      +-----------+----------------------------+
      1 row selected (1.525 seconds)
      
      0: jdbc:drill:zk=local> select * from tmp.t;
      
      Feb 02, 2017 11:24:09 AM org.apache.calcite.sql.validate.SqlValidatorException <init>
      SEVERE: org.apache.calcite.sql.validate.SqlValidatorException: Table 'tmp.t' not found
      Feb 02, 2017 11:24:09 AM org.apache.calcite.runtime.CalciteException <init>
      SEVERE: org.apache.calcite.runtime.CalciteContextException: From line 1, column 15 to line 1, column 17: Table 'tmp.t' not found
      Error: VALIDATION ERROR: From line 1, column 15 to line 1, column 17: Table 'tmp.t' not found
      SQL Query null
      [Error Id: 5266cb67-9d37-4a94-9a4e-28a4a2f94be5 on localhost:31010] (state=,code=0)
      

      Attachments

        Issue Links

          Activity

            githubbot ASF GitHub Bot added a comment -

            GitHub user arina-ielchiieva opened a pull request:

            https://github.com/apache/drill/pull/736

            DRILL-5238: CTTAS: unable to resolve temporary table if workspace is …

            …indicated without schema

            1. Added additional check for passed table is temporary if default workspace is partially indicated (issue is described in Jira DRILL-5238).
            2. Removed `UserSession.getDefaultSchemaName()` method as it contained exactly the same logic as `UserSession.getDefaultSchemaPath()`.
            3. Since `UserSession.properties` are set using builder, they are not required but when not set, they may cause NPE when `UserSession.getProp()` method is called. Moved `UserSession.properties` into constructor.
            4. Unit test for partial schema usage with temporary table.

            You can merge this pull request into a Git repository by running:

            $ git pull https://github.com/arina-ielchiieva/drill DRILL-5238

            Alternatively you can review and apply these changes as the patch at:

            https://github.com/apache/drill/pull/736.patch

            To close this pull request, make a commit to your master/trunk branch
            with (at least) the following in the commit message:

            This closes #736


            commit a787c6c2b49de6b8114f028864ff8d67cc5989be
            Author: Arina Ielchiieva <arina.yelchiyeva@gmail.com>
            Date: 2017-02-02T11:47:19Z

            DRILL-5238: CTTAS: unable to resolve temporary table if workspace is indicated without schema


            githubbot ASF GitHub Bot added a comment - GitHub user arina-ielchiieva opened a pull request: https://github.com/apache/drill/pull/736 DRILL-5238 : CTTAS: unable to resolve temporary table if workspace is … …indicated without schema 1. Added additional check for passed table is temporary if default workspace is partially indicated (issue is described in Jira DRILL-5238 ). 2. Removed `UserSession.getDefaultSchemaName()` method as it contained exactly the same logic as `UserSession.getDefaultSchemaPath()`. 3. Since `UserSession.properties` are set using builder, they are not required but when not set, they may cause NPE when `UserSession.getProp()` method is called. Moved `UserSession.properties` into constructor. 4. Unit test for partial schema usage with temporary table. You can merge this pull request into a Git repository by running: $ git pull https://github.com/arina-ielchiieva/drill DRILL-5238 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/736.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #736 commit a787c6c2b49de6b8114f028864ff8d67cc5989be Author: Arina Ielchiieva <arina.yelchiyeva@gmail.com> Date: 2017-02-02T11:47:19Z DRILL-5238 : CTTAS: unable to resolve temporary table if workspace is indicated without schema
            zfong Zelaine Fong added a comment -

            Assigned Reviewer to jni

            zfong Zelaine Fong added a comment - Assigned Reviewer to jni
            githubbot ASF GitHub Bot added a comment -

            Github user sudheeshkatkam commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99162943

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            — End diff –

            static

            githubbot ASF GitHub Bot added a comment - Github user sudheeshkatkam commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99162943 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { — End diff – static
            githubbot ASF GitHub Bot added a comment -

            Github user arina-ielchiieva commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99163906

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            — End diff –

            `checkForTemporaryTable` is just private method used only in `DrillCalciteCatalogReader` class. I don't think it should be `private static.`

            githubbot ASF GitHub Bot added a comment - Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99163906 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { — End diff – `checkForTemporaryTable` is just private method used only in `DrillCalciteCatalogReader` class. I don't think it should be `private static.`
            githubbot ASF GitHub Bot added a comment -

            Github user sudheeshkatkam commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99164906

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            — End diff –

            To emphasize that this method does not rely on any internal state; it's fine either way.

            githubbot ASF GitHub Bot added a comment - Github user sudheeshkatkam commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99164906 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { — End diff – To emphasize that this method does not rely on any internal state; it's fine either way.
            githubbot ASF GitHub Bot added a comment -

            Github user arina-ielchiieva commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99166435

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            — End diff –

            Makes sense. I'll make a change.

            githubbot ASF GitHub Bot added a comment - Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99166435 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { — End diff – Makes sense. I'll make a change.
            githubbot ASF GitHub Bot added a comment -

            Github user arina-ielchiieva commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99167324

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            — End diff –

            Oh, actually I can't since `DrillCalciteCatalogReader` is inner class and inner classes can't have static declarations

            githubbot ASF GitHub Bot added a comment - Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99167324 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { — End diff – Oh, actually I can't since `DrillCalciteCatalogReader` is inner class and inner classes can't have static declarations
            zfong Zelaine Fong added a comment -

            Changed Reviewer to sudheeshkatkam, since he's provided review comments.

            zfong Zelaine Fong added a comment - Changed Reviewer to sudheeshkatkam , since he's provided review comments.
            githubbot ASF GitHub Bot added a comment -

            Github user paul-rogers commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99388535

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            + if (names.size() == 1)

            { + return true; + }

            — End diff –

            A single name might be a temporary table, or it might not be. Hence the name suggestion above. Is the intent to simply apply a filter so that we separate out those that are not temporary tables from those that may or may not be?

            If we want a positive check that this is, in fact, a temp table, wouldn't we have to resolve the name against known temp tables rather than just infer based on path length?

            And, if we are just filtering out known non-temp tables, is that good enough for the use case? That is, can the case we are fixing handle the ambiguous "may or may not be temporary" case?

            githubbot ASF GitHub Bot added a comment - Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99388535 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { + if (names.size() == 1) { + return true; + } — End diff – A single name might be a temporary table, or it might not be. Hence the name suggestion above. Is the intent to simply apply a filter so that we separate out those that are not temporary tables from those that may or may not be? If we want a positive check that this is, in fact, a temp table, wouldn't we have to resolve the name against known temp tables rather than just infer based on path length? And, if we are just filtering out known non-temp tables, is that good enough for the use case? That is, can the case we are fixing handle the ambiguous "may or may not be temporary" case?
            githubbot ASF GitHub Bot added a comment -

            Github user paul-rogers commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99387926

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            — End diff –

            `checkForTemporaryTable` --> `isTemporaryTable` ?

            Actually, given the first condition, is this `mightBeTemporaryTable`?

            githubbot ASF GitHub Bot added a comment - Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99387926 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { — End diff – `checkForTemporaryTable` --> `isTemporaryTable` ? Actually, given the first condition, is this `mightBeTemporaryTable`?
            githubbot ASF GitHub Bot added a comment -

            Github user arina-ielchiieva commented on a diff in the pull request:

            https://github.com/apache/drill/pull/736#discussion_r99393786

            — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java —
            @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) {
            }
            return super.getTable(names);
            }
            +
            + /**
            + * Check if passed table is temporary or not should be done 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/>
            + *
            + * Examples (where dfs.tmp is default temporary workspace):
            + * <li>select * from t<li/>
            + * <li>select * from dfs.tmp.t<li/>
            + * <li>use dfs; select * from tmp.t<li/>
            + *
            + * @param names list of schema and table names, table name is always the last element
            + * @param defaultSchemaPath current schema path set using USE command
            + * @param drillConfig drill config
            + * @return true if check for temporary table should be done, false otherwise
            + */
            + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) {
            + if (names.size() == 1)

            { + return true; + }

            — End diff –

            Renamed method to `mightBeTemporaryTable`. You are right the intent here is just to filter out cases when we don't need to check is passed table might be temporary. When table is passed to `getTableName` we don't know if it's temporary or not. If it is temporary we need to substitute the name to generated one and there are several indicators when we need to check this: table name is without schema or indicated schema is default temporary workspace,in all other cases we just check regular tables directly.

            githubbot ASF GitHub Bot added a comment - Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/736#discussion_r99393786 — Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java — @@ -527,5 +529,31 @@ public RelOptTableImpl getTable(final List<String> names) { } return super.getTable(names); } + + /** + * Check if passed table is temporary or not should be done 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/> + * + * Examples (where dfs.tmp is default temporary workspace): + * <li>select * from t<li/> + * <li>select * from dfs.tmp.t<li/> + * <li>use dfs; select * from tmp.t<li/> + * + * @param names list of schema and table names, table name is always the last element + * @param defaultSchemaPath current schema path set using USE command + * @param drillConfig drill config + * @return true if check for temporary table should be done, false otherwise + */ + private boolean checkForTemporaryTable(List<String> names, String defaultSchemaPath, DrillConfig drillConfig) { + if (names.size() == 1) { + return true; + } — End diff – Renamed method to `mightBeTemporaryTable`. You are right the intent here is just to filter out cases when we don't need to check is passed table might be temporary. When table is passed to `getTableName` we don't know if it's temporary or not. If it is temporary we need to substitute the name to generated one and there are several indicators when we need to check this: table name is without schema or indicated schema is default temporary workspace,in all other cases we just check regular tables directly.
            githubbot ASF GitHub Bot added a comment -

            Github user asfgit closed the pull request at:

            https://github.com/apache/drill/pull/736

            githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/736

            Merged into master with commit id 1ec3edf01575f302b765b62317ca16c1547dbe10

            arina Arina Ielchiieva added a comment - Merged into master with commit id 1ec3edf01575f302b765b62317ca16c1547dbe10
            khfaraaz Khurram Faraaz added a comment -

            Verified on Drill 1.10.0 commit ID : 300e9349

            apache drill 1.10.0-SNAPSHOT
            "a drill is a terrible thing to waste"
            0: jdbc:drill:schema=dfs.tmp> use dfs;
            +-------+----------------------------------+
            |  ok   |             summary              |
            +-------+----------------------------------+
            | true  | Default schema changed to [dfs]  |
            +-------+----------------------------------+
            1 row selected (0.367 seconds)
            0: jdbc:drill:schema=dfs.tmp> create temporary table tmp.t as select 'A' from (values(1));
            +-----------+----------------------------+
            | Fragment  | Number of records written  |
            +-----------+----------------------------+
            | 0_0       | 1                          |
            +-----------+----------------------------+
            1 row selected (0.486 seconds)
            0: jdbc:drill:schema=dfs.tmp> select * from tmp.t;
            +---------+
            | EXPR$0  |
            +---------+
            | A       |
            +---------+
            1 row selected (0.21 seconds)
            0: jdbc:drill:schema=dfs.tmp>
            
            khfaraaz Khurram Faraaz added a comment - Verified on Drill 1.10.0 commit ID : 300e9349 apache drill 1.10.0-SNAPSHOT "a drill is a terrible thing to waste" 0: jdbc:drill:schema=dfs.tmp> use dfs; +-------+----------------------------------+ | ok | summary | +-------+----------------------------------+ | true | Default schema changed to [dfs] | +-------+----------------------------------+ 1 row selected (0.367 seconds) 0: jdbc:drill:schema=dfs.tmp> create temporary table tmp.t as select 'A' from (values(1)); +-----------+----------------------------+ | Fragment | Number of records written | +-----------+----------------------------+ | 0_0 | 1 | +-----------+----------------------------+ 1 row selected (0.486 seconds) 0: jdbc:drill:schema=dfs.tmp> select * from tmp.t; +---------+ | EXPR$0 | +---------+ | A | +---------+ 1 row selected (0.21 seconds) 0: jdbc:drill:schema=dfs.tmp>

            People

              arina Arina Ielchiieva
              arina Arina Ielchiieva
              Sudheesh Katkam Sudheesh Katkam
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: