Vuk Ercegovac 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 9: (15 comments) http://gerrit.cloudera.org:8080/#/c/9065/9/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/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@513 PS9, Line 513: l_linenumber INT, " : + "l_quantity DECIMAL(12,2), " : + "l_extendedprice DECIMAL(12,2), " : + "l_discount DECIMAL(12,2), " : + "l_tax DECIMAL(12,2), " : + "l_returnf are all of these columns needed? lets try to get the simplest table that exercises the condition you're interested in. http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@545 PS9, Line 545: // the row number, the FULL OUTER JOIN should have >=Long.MAX_VALUE rows nit: add a space. http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@546 PS9, Line 546: test guards against the cardinality simplify: "tests for" http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@567 PS9, Line 567: should be is http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@567 PS9, Line 567: // create an empty table and cardinality should be 0 test that http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@574 PS9, Line 574: should not be is not http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@574 PS9, Line 574: // create a table with negative rows and cardinality should not be negative test that http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@582 PS9, Line 582: //This test guards against the cardinality estimate in SubplanNode.java format this comment like the others. http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@587 PS9, Line 587: "int_array_array ARRAY<ARRAY<INT>>, " : + "int_map MAP<STRING,INT>, " : + "int_map_array ARRAY<MAP<STRING,INT>>, " : + "nested_struct STRUCT<a:INT,b:ARRAY<INT>,c:" : + "STRUCT<d: is it possible to simplify this? http://gerrit.cloudera.org:8080/#/c/9065/9/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/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@783 PS9, Line 783: boundary nit: bounds http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@785 PS9, Line 785: LUE (9223372036854775807), then the join of these two tables : * can have cardinality greater than Long.MAX_VALUE, which is overflow, : * instead, 9223372036854775807 should be shown as the cardinality : * in the explain message good example, pls rephrase it. perhaps: ", ... then the join of these two tables may have an estimated cardinality of > Long.MAX_VALUE, which is an overflow. http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@790 PS9, Line 790: tablename is the table the query is requested on. : * The parameter zeroCardinality is a boolean value, if it is true : * then the cardinality should be zero perhaps its easier if you provide explicit, expected lower/upper bounds as parameters. so the test for overflow would be: [Long.MAX_VALUE, Long.MAX_VALUE] the test for zero could be [0, 0]. the test for a non-negative Long would be [0, Long.MAX_VALUE] http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@802 PS9, Line 802: queryCtx.client_request.setStmt("compute stats " + tablename); : queryCtx.client_request.query_options = defaultQueryOptions(); what does this do? http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@806 PS9, Line 806: t.setStmt(query); since the query is set after calling the frontend (L805), is the plan that is traversed on L813 for the query or the "compute stats"? http://gerrit.cloudera.org:8080/#/c/9065/9/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@827 PS9, Line 827: } as a debugging aid, you might try to split this method into two: one that extracts cardinalities (and the node associated with each cardinality), and a bounds check applied to each found cardinality. then you can perhaps more easily inspect the plan that we're dealing with here and the cardinalities that are present (if any). -- 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: 9 Gerrit-Owner: 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: xyutin...@cloudera.com Gerrit-Comment-Date: Tue, 06 Feb 2018 18:32:56 +0000 Gerrit-HasComments: Yes
