Jim Apple has posted comments on this change.

Change subject: IMPALA-1654: Support general predicates in most partition DDL 
operations.
......................................................................


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/1563/3/fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java:

Line 232:       }
> grep -iIrF 'fully bounded' fe/src/
Please explain this more. Right now it still uses this "fully bounded" wording.


http://gerrit.cloudera.org:8080/#/c/1563/6/fe/src/main/java/com/cloudera/impala/planner/HdfsPartitionPruner.java
File fe/src/main/java/com/cloudera/impala/planner/HdfsPartitionPruner.java:

Line 91:    * If 'allowEmpty' is True, partitions that contain no file may be 
survived
"be survived during" -> "survive"


http://gerrit.cloudera.org:8080/#/c/1563/6/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:

Line 515: alter table t_part drop partition (j=2, s is null)
Why drop the empty string case? Does that test no longer work?


http://gerrit.cloudera.org:8080/#/c/1563/3/testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
File 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test:

Line 316: compute incremental stats incremental_null_part_key partition(p is 
NULL);
Does this test no longer work as written? We must be very careful about 
breaking compatibility.


http://gerrit.cloudera.org:8080/#/c/1563/6/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates.test
File 
testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates.test:

Line 19: # Add a partition
"some partitions"


Line 60: alter table p1 partition (j<2 and j>0, k<>4) set uncached
Can you also so a test with an "or" in one of the comma-separated clauses?


-- 
To view, visit http://gerrit.cloudera.org:8080/1563
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Amos Bird <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Amos Bird <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-HasComments: Yes

Reply via email to