Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11814 )
Change subject: IMPALA-7586: fix predicate pushdown of escaped strings ...................................................................... Patch Set 6: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/11814/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java File fe/src/main/java/org/apache/impala/planner/KuduScanNode.java: http://gerrit.cloudera.org:8080/#/c/11814/5/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java@319 PS5, Line 319: getExplainString > The behaviour here seems ok to me - it calls toSql(), which consistently es The behavior overall seems ok to me as well. But in this case, it diverged from the original conjunct that was used to push down. http://gerrit.cloudera.org:8080/#/c/11814/5/testdata/data/strings_with_quotes.csv File testdata/data/strings_with_quotes.csv: http://gerrit.cloudera.org:8080/#/c/11814/5/testdata/data/strings_with_quotes.csv@11 PS5, Line 11: foo\\"bar,11 > Done. I assume you meant that you wanted the value in the table in include Yep. http://gerrit.cloudera.org:8080/#/c/11814/5/testdata/workloads/functional-query/queries/QueryTest/string-escaping-rcfile-bug.test File testdata/workloads/functional-query/queries/QueryTest/string-escaping-rcfile-bug.test: http://gerrit.cloudera.org:8080/#/c/11814/5/testdata/workloads/functional-query/queries/QueryTest/string-escaping-rcfile-bug.test@4 PS5, Line 4: ==== > I think it's better to exercise and track the known-bad behaviour than leav Agreed. -- To view, visit http://gerrit.cloudera.org:8080/11814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I53d6e20dd48ab6837ddd325db8a9d49ee04fed28 Gerrit-Change-Number: 11814 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 30 Oct 2018 21:24:44 +0000 Gerrit-HasComments: Yes
