Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/8818 )
Change subject: IMPALA-3942: Fix wronly escaped string literal in front-end ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/8818/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java: http://gerrit.cloudera.org:8080/#/c/8818/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@120 PS5, Line 120: sb.append("\""); Just asking: Wouldn't this be more readable to say sb.append('"') (double quote surrounded by single quotes) same as the row above? Or can't you append nextChar here? If the answer is yes for the last question then it might seem reasonable to rework this if-else: if (nextchar is not doublequote) { append(currentChar) } append(nextchar) http://gerrit.cloudera.org:8080/#/c/8818/5/tests/query_test/test_queries.py File tests/query_test/test_queries.py: http://gerrit.cloudera.org:8080/#/c/8818/5/tests/query_test/test_queries.py@272 PS5, Line 272: def prepare(self, unique_database): nit: as the only usage of prepare() is in the last test of this file, I'd move this function closer to it. http://gerrit.cloudera.org:8080/#/c/8818/5/tests/query_test/test_queries.py@297 PS5, Line 297: result = self.execute_query("select '\\\''") I'm a bit confused of all these backslashes and single-double quotes to be honest :), but shouldn't this query return: "\'" instead of "'"? -- To view, visit http://gerrit.cloudera.org:8080/8818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec Gerrit-Change-Number: 8818 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 09 Jan 2018 10:24:11 +0000 Gerrit-HasComments: Yes
