Dimitris Tsirogiannis has posted comments on this change. ( http://gerrit.cloudera.org:8080/8890 )
Change subject: IMPALA-3526: update FE tests to pass on S3 ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/8890/3/fe/src/test/java/org/apache/impala/planner/PlannerTest.java File fe/src/test/java/org/apache/impala/planner/PlannerTest.java: http://gerrit.cloudera.org:8080/#/c/8890/3/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@130 PS3, Line 130: @Test : public void testJoinOrder() { : runPlannerTestFile("join-order"); : } : : @Test : public void testJoinOrderHdfs() { : runPlannerTestFile("join-order-hdfs"); : } Just by reading this section, it's not clear what we're testing in join-order vs join-order-hdfs (same comment applies to the s3planner tests). Maybe add a comment to explain what is going on with these tests and why the separation is needed. Also, another thought would be to rename join-order to join-order-all-fs or join-order-common (or something along these lines). http://gerrit.cloudera.org:8080/#/c/8890/3/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test File testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test: http://gerrit.cloudera.org:8080/#/c/8890/3/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test@1 PS3, Line 1: # TPCH-Q1 Shouldn't this be renamed to tpch.test to be consistent with the other renames you did? -- To view, visit http://gerrit.cloudera.org:8080/8890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c8221949e76b0a0b9192e6b56c4da5eeae04141 Gerrit-Change-Number: 8890 Gerrit-PatchSet: 3 Gerrit-Owner: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 10 Jan 2018 21:08:16 +0000 Gerrit-HasComments: Yes
