Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14036 )
Change subject: IMPALA-8848: fix UNION missing input cardinality bug ...................................................................... Patch Set 2: (4 comments) lgtm, just a bunch of clarifying questions and nits. http://gerrit.cloudera.org:8080/#/c/14036/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14036/2//COMMIT_MSG@9 PS2, Line 9: If a UNION has input of unknown size, we should nit: I got confused by this statement for a minute. Rephrase may be? (to clarify that atleast one child with a valid cardinality is needed) http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java File fe/src/main/java/org/apache/impala/planner/UnionNode.java: http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java@124 PS2, Line 124: nit:..atleast..? http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/main/java/org/apache/impala/planner/UnionNode.java@127 PS2, Line 127: cardinality_ = checkedAdd(totalChildCardinality, constExprLists_.size()); nit:Preconditions.check(constExprLists_.size() > 0)? http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java File fe/src/test/java/org/apache/impala/planner/CardinalityTest.java: http://gerrit.cloudera.org:8080/#/c/14036/2/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java@724 PS2, Line 724: path, UnionNode.class); Should we also add a test to cover something like select * from tbl_has_valid_cards union select * from tbl_without_valid_cards ? -- To view, visit http://gerrit.cloudera.org:8080/14036 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic3ed670ffb685d8ff24824933ca303f3219737bb Gerrit-Change-Number: 14036 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Fri, 09 Aug 2019 16:25:06 +0000 Gerrit-HasComments: Yes
