Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )
Change subject: IMPALA-5821: Add query with implicit casts to extended explain output. ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG@51 PS4, Line 51: "" This quote seems strange here - is it included in the output? http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG@69 PS4, Line 69: 80 Isn't it 60? http://gerrit.cloudera.org:8080/#/c/11719/4/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/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@772 PS4, Line 772: if (line.contains("Analyzed query:")) { : builder.append(line).append("\n"); : inImplictCasts = true; : } else if (inImplictCasts) { : // Keep copying the query with implicit casts. : // This works because this is the last thing in the header. : builder.append(line).append("\n"); : } optional: this could be expressed in a more compact way: inImplictCasts |= line.contains("Analyzed query:"); if (inImplictCasts) builder.append(line).append("\n"); http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java File fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java: http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@83 PS4, Line 83: wrap length for testWrapText() - less than 80 to make test layout nicer nit: Capital W + . at the end. http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@89 PS4, Line 89: * nit: extra * http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@127 PS4, Line 127: * Check that code that has been wrapped is correctly formatted nit: missing . -- To view, visit http://gerrit.cloudera.org:8080/11719 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0 Gerrit-Change-Number: 11719 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Sherman <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Wed, 31 Oct 2018 15:31:26 +0000 Gerrit-HasComments: Yes
