Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8238 )
Change subject: IMPALA-4524: Batch ALTER TABLE...ADD PARTITION calls. ...................................................................... Patch Set 8: (4 comments) Thanks for the review, Alex! I believe I addressed your comments. I've re-run test_alter_table_create_many_partitions (the new test). I'm kicking off a full test run (https://jenkins.impala.io/view/Utility/job/pre-review-test/68/) to make sure my Lists.partition() change wasn't fat-fingered somehow. LD_LIBRARY_PATH=./toolchain/kudu-bec2a24/release/lib:$LD_LIBRARY_PATH infra/python/env/bin/py.test tests/metadata/test_ddl.py -k test_alter_table_create_many_partitions -s http://gerrit.cloudera.org:8080/#/c/8238/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/8238/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1958 PS7, Line 1958: Lists.partition(allHmsPartitionsToAdd, MAX_PARTITION_UPDATES_PER_RPC)) { > While you are here, can you also change bulkAlterPartitions() to use Lists. Done http://gerrit.cloudera.org:8080/#/c/8238/7/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: http://gerrit.cloudera.org:8080/#/c/8238/7/tests/metadata/test_ddl.py@514 PS7, Line 514: service.wait_for_metric_value(class_cache_misses_metric, class_cache_misses) > This test doesn't belong in the TestLibCache class. I moved it up in this faile in test_ddl. Good catch that this file had multiple classes; I missed that somehow. I found a test test_hms_integration.py:test_add_overlapping_partitions that does have multi-partition add alter, but it's all about Hive/Impala compatibility, so I think test_ddl.py is a slightly better home here. I'm very happy to defer to you. http://gerrit.cloudera.org:8080/#/c/8238/7/tests/metadata/test_ddl.py@519 PS7, Line 519: cache. create_stmts is the list of CREATE statements to be executed in order > "use" can generally cause problems in tests because we sometimes run a test Done http://gerrit.cloudera.org:8080/#/c/8238/7/tests/metadata/test_ddl.py@527 PS7, Line 527: self.client.set_configuration(vector.get_value("exec_option")) > This comment can become stale quickly. I suggest not adding it an output ro Done -- To view, visit http://gerrit.cloudera.org:8080/8238 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95f8221ff08c0f126f951f7d37ff5e57985f855f Gerrit-Change-Number: 8238 Gerrit-PatchSet: 8 Gerrit-Owner: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Mon, 23 Oct 2017 17:20:28 +0000 Gerrit-HasComments: Yes
