Amos Bird has posted comments on this change. Change subject: IMPALA-1654: Support general predicates in most partition DDL operations. ......................................................................
Patch Set 3: (7 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 55: private Boolean partitionShouldExist_; > yes please Done Line 58: private boolean isKV_ = false; > ", because only they require partition specs in a particular KV form. See P Done Line 232: "Partition expr is not applicable. It should be bounded by " + > Please explain this more. Right now it still uses this "fully bounded" word sorry it's 'fully bound' in the orginal src :-). I've add more explanations. Hope it's clear now. 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: */ > "be survived during" -> "survive" Done 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: ---- QUERY > Why drop the empty string case? Does that test no longer work? No, it doesn't. NULL partition can only be choosen by 'is null' 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 > "some partitions" Done Line 60 > Can you also so a test with an "or" in one of the comma-separated clauses? Done -- 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: 3 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
