Xinran Tinney has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9065 )

Change subject: IMPALA-5440 Add planner tests with extreme statistics values
......................................................................


Patch Set 13:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/9065/12/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/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@527
PS12, Line 527: t m
> nit: you have three tables in the query.
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@527
PS12, Line 527: tests that multiplying the input cardinalities does not overflow
              :     // the cross-join's estimated cardinality
              :     String query = "select * from tpch.cardinality_overflow
> perhaps the following template is easier? if you think its better, pls use
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@550
PS12, Line 550:         + "< b.l_orderkey";
> nit: space
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@564
PS12, Line 564: = '-
> nit: row count
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@783
PS12, Line 783: query, wh
> replace with: 'query'
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@784
PS12, Line 784:    * cardinality is calculated based on, extracts the 
cardinalities
> add: ['min', 'max']
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@785
PS12, Line 785: from plan nodes and fails if the cardinality is not within the
              :    * specified bounds [min, max].
              :    */
> Done
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@793
PS12, Line 793: StringBuilder explainBui
> move the declaration and assignment to L796 (no need to split this over two
Done


http://gerrit.cloudera.org:8080/#/c/9065/12/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@798
PS12, Line 798: || execRequest.query_ex
> from my reading of createExecRequest, this will never be null, so remove th
Done



--
To view, visit http://gerrit.cloudera.org:8080/9065
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86dec47cf1438882cafaec53e97864ccfcdff3cb
Gerrit-Change-Number: 9065
Gerrit-PatchSet: 13
Gerrit-Owner: Xinran Tinney <xyutin...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com>
Gerrit-Reviewer: Xinran Tinney <xyutin...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Feb 2018 18:50:32 +0000
Gerrit-HasComments: Yes

Reply via email to