Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-1670

Support multiple partition specs in ALTER TABLE ADD PARTITION

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Impala 2.1
    • Fix Version/s: Impala 2.9.0
    • Component/s: Catalog
    • Labels:

      Description

      Hive supports multiple partition specs in ALTER TABLE:

      ALTER TABLE tbl ADD PARTITION (p1=1,p2=1), (p1=2,p2=1),(p1=1, p2=2);

        Activity

        Hide
        attilaj Attila Jeges added a comment -

        If we follow the Hive syntax, than the format of the statement should be as follows:

        ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION
        partition_spec [LOCATION 'location1'] [{CACHED IN 'pool1' | UNCACHED}] 
        partition_spec [LOCATION 'location2'] [{CACHED IN 'pool2' | UNCACHED}] ...;
        

        where partition_spec is:

        (partition_column = partition_col_value, partition_column = partition_col_value, ...)
        

        So there is an optional location and cache clause following each partition specification. Furthermore, there are no commas between the partition specifications.
        Is this correct?

        Show
        attilaj Attila Jeges added a comment - If we follow the Hive syntax, than the format of the statement should be as follows: ALTER TABLE table_name ADD [IF NOT EXISTS] PARTITION partition_spec [LOCATION 'location1'] [{CACHED IN 'pool1' | UNCACHED}] partition_spec [LOCATION 'location2'] [{CACHED IN 'pool2' | UNCACHED}] ...; where partition_spec is: (partition_column = partition_col_value, partition_column = partition_col_value, ...) So there is an optional location and cache clause following each partition specification. Furthermore, there are no commas between the partition specifications. Is this correct?
        Hide
        kwho Michael Ho added a comment -

        Yes, we should adhere to the syntax of Hive. Our existing syntax for alter table is already very similar to Hive's if not identical:
        http://www.cloudera.com/documentation/archive/impala/2-x/2-1-x/topics/impala_alter_table.html

        I think the spec of partition_spec above is a bit off and it's missing the PARTITION keyword.
        The syntax we want to use is like:

        ALTER TABLE page_view ADD PARTITION (dt='2008-08-08', country='us') location '/path/to/us/part080808' PARTITION (dt='2008-08-09', country='us') location '/path/to/us/part080809';
        

        Please also watch up for conflict with an in-flight change: https://gerrit.cloudera.org/#/c/1563/

        Show
        kwho Michael Ho added a comment - Yes, we should adhere to the syntax of Hive. Our existing syntax for alter table is already very similar to Hive's if not identical: http://www.cloudera.com/documentation/archive/impala/2-x/2-1-x/topics/impala_alter_table.html I think the spec of partition_spec above is a bit off and it's missing the PARTITION keyword. The syntax we want to use is like: ALTER TABLE page_view ADD PARTITION (dt='2008-08-08', country='us') location '/path/to/us/part080808' PARTITION (dt='2008-08-09', country='us') location '/path/to/us/part080809'; Please also watch up for conflict with an in-flight change: https://gerrit.cloudera.org/#/c/1563/
        Hide
        attilaj Attila Jeges added a comment -

        I see.

        What about the IF NOT EXISTS clause? Is the following correct?

        ALTER TABLE page_view ADD IF NOT EXISTS PARTITION (dt='2008-08-08', country='us') location '/path/to/us/part080808' 
                                 IF NOT EXISTS PARTITION (dt='2008-08-09', country='us') location '/path/to/us/part080809';
        

        Or the statement may contain only one IF NOT EXISTS clause that applies to all the partitions?

        ALTER TABLE page_view ADD IF NOT EXISTS PARTITION (dt='2008-08-08', country='us') location '/path/to/us/part080808' 
                                           PARTITION (dt='2008-08-09', country='us') location '/path/to/us/part080809';
        

        I think, Hive accepts only the latter syntax.

        Show
        attilaj Attila Jeges added a comment - I see. What about the IF NOT EXISTS clause? Is the following correct? ALTER TABLE page_view ADD IF NOT EXISTS PARTITION (dt='2008-08-08', country='us') location '/path/to/us/part080808' IF NOT EXISTS PARTITION (dt='2008-08-09', country='us') location '/path/to/us/part080809'; Or the statement may contain only one IF NOT EXISTS clause that applies to all the partitions? ALTER TABLE page_view ADD IF NOT EXISTS PARTITION (dt='2008-08-08', country='us') location '/path/to/us/part080808' PARTITION (dt='2008-08-09', country='us') location '/path/to/us/part080809'; I think, Hive accepts only the latter syntax.
        Hide
        kwho Michael Ho added a comment -

        The latter one I suppose. Please confirm the behavior on Hive.

        Show
        kwho Michael Ho added a comment - The latter one I suppose. Please confirm the behavior on Hive.
        Hide
        attilaj Attila Jeges added a comment -

        Confirmed, Hive supports the latter syntax.

        Show
        attilaj Attila Jeges added a comment - Confirmed, Hive supports the latter syntax.
        Hide
        dtsirogiannis Dimitris Tsirogiannis added a comment -

        Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750
        Reviewed-on: http://gerrit.cloudera.org:8080/4144
        Reviewed-by: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
        Tested-by: Impala Public Jenkins

        M common/thrift/JniCatalog.thrift
        M fe/src/main/cup/sql-parser.cup
        M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
        A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
        M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java
        M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
        M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
        M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
        M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
        M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
        M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
        M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
        M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
        M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
        M tests/common/impala_test_suite.py
        M tests/metadata/test_hms_integration.py
        M tests/metadata/test_refresh_partition.py
        17 files changed, 858 insertions, 207 deletions

        Approvals:
        Impala Public Jenkins: Verified
        Dimitris Tsirogiannis: Looks good to me, approved

        Show
        dtsirogiannis Dimitris Tsirogiannis added a comment - Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Reviewed-on: http://gerrit.cloudera.org:8080/4144 Reviewed-by: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com> Tested-by: Impala Public Jenkins — M common/thrift/JniCatalog.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java A fe/src/main/java/org/apache/impala/analysis/PartitionDef.java M fe/src/main/java/org/apache/impala/analysis/PartitionKeyValue.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test M tests/common/impala_test_suite.py M tests/metadata/test_hms_integration.py M tests/metadata/test_refresh_partition.py 17 files changed, 858 insertions , 207 deletions Approvals: Impala Public Jenkins: Verified Dimitris Tsirogiannis: Looks good to me, approved

          People

          • Assignee:
            attilaj Attila Jeges
            Reporter:
            henryr Henry Robinson
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development