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 5: Code-Review+1 (3 comments) A bunch of minor comments. Fix lgtm. 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 It looks like this always prints the normalized/unescaped string even though we used the original string for predicates in the scan nodes. We could've probably diagnosed the issue faster if this was right. 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: add foo\"bar, 11 too? That shouldn't be returned with ..where s = "foo\"bar".. 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: # IMPALA-7778: escapes are ignored so output is incorrect Can't we just xfail if format == 'rc' instead of doing this? or is it because if someone fixes it in the future, this test starts failing and they know it right away? -- 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: 5 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: Tue, 30 Oct 2018 17:38:19 +0000 Gerrit-HasComments: Yes
