Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12136 )
Change subject: IMPALA-8021: Add estimated cardinality to EXPLAIN output ...................................................................... Patch Set 12: (2 comments) A couple of clarifications after looking at the TestUtils code again. http://gerrit.cloudera.org:8080/#/c/12136/12/fe/src/test/java/org/apache/impala/testutil/TestUtils.java File fe/src/test/java/org/apache/impala/testutil/TestUtils.java: http://gerrit.cloudera.org:8080/#/c/12136/12/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@207 PS12, Line 207: If found, then parse the values and compare them. Report a match : * if the values are within 5%, else report a mismatch. > Is this because of the loss of information during pretty printing and then Actually, looking at this again, it appears that we are just comparing the pretty printed cardinalities (actual and expected). So, my understanding above is wrong I think. I'm wondering why we see values that are 5% off. Do some tests fail without this? http://gerrit.cloudera.org:8080/#/c/12136/12/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@226 PS12, Line 226: SKIP > you mean DIFF? (same below) To be clear, if something is unparseable, I'm guessing something went wrong or is this for handling (cardinality = unavailable) part? -- To view, visit http://gerrit.cloudera.org:8080/12136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9aa2d715b04cbb279aaffec8c5692686562d986 Gerrit-Change-Number: 12136 Gerrit-PatchSet: 12 Gerrit-Owner: Paul Rogers <prog...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Sat, 05 Jan 2019 04:09:12 +0000 Gerrit-HasComments: Yes
