Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <201611150327.uAF3Ra6b020798@ip-10-146-233-104.ec2.internal>
References: <gerrit.1470906636000.I2c9162fcf9d227b8daf4c2e761d57bab4e26408f@gerrit.cloudera.org>
 <201611150327.uAF3Ra6b020798@ip-10-146-233-104.ec2.internal>
From: Alex Behm <alex.behm@cloudera.com>
Date: Mon, 14 Nov 2016 21:31:39 -0800
Message-ID: <CAMvjXi8Jo=T8VLeNWfrVZNmjnVM2-_y2dnuJhEWp2VmFj8GKjA@mail.gmail.com>
Subject: Re: [Impala-ASF-CR] IMPALA-1654: General partition exprs in DDL operations.
To: Amos Bird <amosbird@gmail.com>, impala-cr <impala-cr@cloudera.com>,
	reviews@impala.incubator.apache.org
Content-Type: multipart/alternative; boundary=001a1134e65866a8af054150451c
archived-at: Tue, 15 Nov 2016 05:31:56 -0000

--001a1134e65866a8af054150451c
Content-Type: text/plain; charset=UTF-8

Amos, thanks a lot for your patience and continued effort on this sizeable
contribution! We're all looking forward to reviewing more of your patches
:-)


On Mon, Nov 14, 2016 at 7:27 PM, Internal Jenkins (Code Review) <
gerrit@cloudera.org> wrote:

> Internal Jenkins has submitted this change and it was merged.
>
> Change subject: IMPALA-1654: General partition exprs in DDL operations.
> ......................................................................
>
>
> IMPALA-1654: General partition exprs in DDL operations.
>
> This commit handles partition related DDL in a more general way. We can
> now use compound predicates to specify a list of partitions in
> statements like ALTER TABLE DROP PARTITION and COMPUTE INCREMENTAL
> STATS, etc. It will also make sure some statements only accept one
> partition at a time, such as PARTITION SET LOCATION and LOAD DATA. ALTER
> TABLE ADD PARTITION remains using the old PartitionKeyValue's logic.
>
> The changed partition related DDLs are as follows,
>
> Table: p (i int) partitioned by (j int, k string)
> Partitions:
> +-------+---+-------+--------+------+--------------+-------------------+
> | j     | k | #Rows | #Files | Size | Bytes Cached | Cache Replication |
> +-------+---+-------+--------+------+--------------+-------------------+
> | 1     | a | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | 1     | b | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | 1     | c | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | 2     | d | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | 2     | e | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | 2     | f | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | Total |   | -1    | 0      | 0B   | 0B           |                   |
> +-------+---+-------+--------+------+--------------+-------------------+
>
> 1. show files in p partition (j<2, k='a');
> 2. alter table p partition (j<2, k in ("b","c") set cached in 'testPool';
>
> // j can appear more than once,
> 3.1. alter table p partition (j<2, j>0, k<>"d") set uncached;
> // it is the same as
> 3.2. alter table p partition (j<2 and j>0, not k="e") set uncached;
> // we can also do 'or'
> 3.3. alter table p partition (j<2 or j>0, k like "%") set uncached;
>
> // missing 'k' matches all values of k
> 4. alter table p partition (j<2) set fileformat textfile;
> 5. alter table p partition (k rlike ".*") set serdeproperties ("k"="v");
> 6. alter table p partition (j is not null) set tblproperties ("k"="v");
> 7. alter table p drop partition (j<2);
> 8. compute incremental stats p partition(j<2);
>
> The remaining old partition related DDLs are as follows,
>
> 1. load data inpath '/path/from' into table p partition (j=2, k="d");
> 2. alter table p add partition (j=2, k="g");
> 3. alter table p partition (j=2, k="g") set location '/path/to';
> 4. insert into p partition (j=2, k="g") values (1), (2), (3);
>
> General partition expressions or partially specified partition specs
> allows partition predicates to return empty partition set no matter
> 'IF EXISTS' is specified.
>
> Examples:
>
> [localhost.localdomain:21000] >
> alter table p drop partition (j=2, k="f");
> Query: alter table p drop partition (j=2, k="f")
> +-------------------------+
> | summary                 |
> +-------------------------+
> | Dropped 1 partition(s). |
> +-------------------------+
> Fetched 1 row(s) in 0.78s
> [localhost.localdomain:21000] >
> alter table p drop partition (j=2, k<"f");
> Query: alter table p drop partition (j=2, k<"f")
> +-------------------------+
> | summary                 |
> +-------------------------+
> | Dropped 2 partition(s). |
> +-------------------------+
> Fetched 1 row(s) in 0.41s
> [localhost.localdomain:21000] >
> alter table p drop partition (k="a");
> Query: alter table p drop partition (k="a")
> +-------------------------+
> | summary                 |
> +-------------------------+
> | Dropped 1 partition(s). |
> +-------------------------+
> Fetched 1 row(s) in 0.25s
> [localhost.localdomain:21000] > show partitions p;
> Query: show partitions p
> +-------+---+-------+--------+------+--------------+-------------------+
> | j     | k | #Rows | #Files | Size | Bytes Cached | Cache Replication |
> +-------+---+-------+--------+------+--------------+-------------------+
> | 1     | b | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | 1     | c | -1    | 0      | 0B   | NOT CACHED   | NOT CACHED        |
> | Total |   | -1    | 0      | 0B   | 0B           |                   |
> +-------+---+-------+--------+------+--------------+-------------------+
> Fetched 3 row(s) in 0.01s
>
> Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
> Reviewed-on: http://gerrit.cloudera.org:8080/3942
> Reviewed-by: Alex Behm <alex.behm@cloudera.com>
> Tested-by: Internal Jenkins
> ---
> M be/src/service/query-exec-state.cc
> M be/src/service/query-exec-state.h
> M common/thrift/CatalogService.thrift
> M common/thrift/Frontend.thrift
> M common/thrift/JniCatalog.thrift
> M fe/src/main/cup/sql-parser.cup
> M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.
> java
> M fe/src/main/java/org/apache/impala/analysis/AlterTableSetCachedStmt.java
> M fe/src/main/java/org/apache/impala/analysis/AlterTableSetFileFormatStmt.
> java
> M fe/src/main/java/org/apache/impala/analysis/
> AlterTableSetLocationStmt.java
> M fe/src/main/java/org/apache/impala/analysis/AlterTableSetStmt.java
> M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.
> java
> M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
> M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
> M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
> M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
> M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
> M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
> M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
> A fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
> M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
> A fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
> M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
> M fe/src/main/java/org/apache/impala/analysis/TableRef.java
> M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
> M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
> M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
> M fe/src/main/java/org/apache/impala/catalog/Table.java
> M fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
> M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
> M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
> M fe/src/main/java/org/apache/impala/service/Frontend.java
> M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
> M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
> M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
> M shell/impala_client.py
> M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
> M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
> M testdata/workloads/functional-query/queries/QueryTest/hdfs-caching.test
> M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
> A testdata/workloads/functional-query/queries/QueryTest/
> partition-ddl-predicates.test
> M tests/metadata/test_ddl.py
> 42 files changed, 1,398 insertions(+), 596 deletions(-)
>
> Approvals:
>   Internal Jenkins: Verified
>   Alex Behm: Looks good to me, approved
>
>
>
> --
> To view, visit http://gerrit.cloudera.org:8080/3942
> To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
>
> Gerrit-MessageType: merged
> Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
> Gerrit-PatchSet: 22
> Gerrit-Project: Impala-ASF
> Gerrit-Branch: master
> Gerrit-Owner: Amos Bird <amosbird@gmail.com>
> Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
> Gerrit-Reviewer: Amos Bird <amosbird@gmail.com>
> Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
> Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
> Gerrit-Reviewer: Internal Jenkins
> Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
> Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
>
> --
> You received this message because you are subscribed to the Google Groups
> "impala-cr" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to impala-cr+unsubscribe@cloudera.com.
> For more options, visit https://groups.google.com/a/cloudera.com/d/optout.
>

--001a1134e65866a8af054150451c--