Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8508 )
Change subject: IMPALA-5237: Support a quoted string in date/time format ...................................................................... Patch Set 10: (1 comment) Sorry for the slow turnaround time here - things got crazy for a while. http://gerrit.cloudera.org:8080/#/c/8508/9/be/src/runtime/timestamp-parse-util.cc File be/src/runtime/timestamp-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/8508/9/be/src/runtime/timestamp-parse-util.cc@175 PS9, Line 175: // e.g. '''foo''bar''' => 'foo'bar' > Let me provide a more intuitive algorithm to find escaped single quotes bet I think we still need a comment like my above one, since we are still treating the last single quote found as the start of a new string. To be honest I think the new approach less intuitive, because we still split the literal token up into multiple tokens if there are escaped quotes in the middle of the string. It seems like the approach only changes when dealing with quotes at the end of the string. E.g. my understanding is that "'foo''bar'" is still broken into two tokens, "foo'" and "bar", whereas "foobar'''" is handled as a single token "foo'". I think fundamentally we need to do this token-splitting because of the memory management - we can't convert "'foo''bar'" into a single token 'foo'bar' without creating a new string and copying the data. So my preference is to use the previous approach and explain it clearly. -- To view, visit http://gerrit.cloudera.org:8080/8508 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie34055ac695748bcfb110bfa6ed5308f469ea178 Gerrit-Change-Number: 8508 Gerrit-PatchSet: 10 Gerrit-Owner: Kim Jin Chul <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Kim Jin Chul <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sat, 03 Mar 2018 01:09:53 +0000 Gerrit-HasComments: Yes
