impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3526: update FE tests to pass on S3
Date Wed, 10 Jan 2018 21:08:16 GMT
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 <vercegovac@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercegovac@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jan 2018 21:08:16 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message