Alex Behm has posted comments on this change. Change subject: IMPALA-1654: Partition expr in DDL operations. ......................................................................
Patch Set 6: (22 comments) We're getting close to getting this merged! http://gerrit.cloudera.org:8080/#/c/3942/6//COMMIT_MSG Commit Message: PS6, Line 7: General partition exprs in DDL opreations. Line 10: now use compound predicates to specify a list of partitions in statement statements Line 56: The only senario that allows partition predicates to return empty Update since we've slightly changed the behavior. http://gerrit.cloudera.org:8080/#/c/3942/6/fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/com/cloudera/impala/analysis/AnalyzeDDLTest.java: Line 106: // Data types don't match Remove this comment and move the test right under the previous one. In general, please cluster these tests by "test scenario" and then add the series of add/drop tests in that cluster. The tests that are specific to only add or only drop can come at the end. Line 127: // Data types don't match remove, already tested above Line 157: // NULL partition keys Can this go in the loop above? Should be the same for add and drop. Line 165: // Data types don't match remove, already tested above Line 185: AnalysisError("alter table functional.alltypes drop " + Do these really return an error? Seems like they shouldn't. Line 204: // IF EXISTS properly checks for partition existence with a fully specified partition. Line 215: AnalyzesOk("alter table functional.alltypes drop " + Add a negative test case with a constant partition expr. Line 217: AnalyzesOk("alter table functional.alltypes drop " + also add a variant of this test that has PARTITION(year>9050 and month=10) to show that the syntax is valid also Line 219: AnalyzesOk("alter table functional.alltypes drop " + also add a test that shows something like this is ok: PARTITION(month + 2000 = year) Line 221: AnalyzesOk("alter table functional.alltypes drop if exists " + Add the expected warning as a param to AnalyzesOK() Line 224: // Not a valid column remove, already tested above http://gerrit.cloudera.org:8080/#/c/3942/6/testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates.test File testdata/workloads/functional-query/queries/QueryTest/partition-ddl-predicates.test: Line 3: use ddl_test_db remove, we're using the unique_database Line 7: # First create an partitioned table a partitioned table Line 11: ---- QUERY no need for this, should be clear that there are no partitions Line 20: alter table p1 add partition (j=1,k="a"); you can move these directly under the create table after L8 Line 22: alter table p1 add partition (j=1,k="c"); add partitions with NULLs and some tests Line 42: show files in p1 partition (j<2, k="a") let's insert some data so there is something to show here Line 144: alter table p1 drop partition (j=2, k="bla") remove, already covered by analyzer test Line 151: '2','d',-1,0,regex:.+,regex:.+,regex:.+,regex:.+,regex:.+,regex:.*/test-warehouse/ddl_test_db.db/p1/j=2/k=d leave the format column since you've altered the format of some partitions -- To view, visit http://gerrit.cloudera.org:8080/3942 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Amos Bird <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Amos Bird <[email protected]> Gerrit-HasComments: Yes
