Kim Jin Chul 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 7: (5 comments) Applied the update. @Gabor, I am sorry to make you review it again after a long time. Now we can deliver some test cases for the mix of single and double quotes as you advised! http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.h File be/src/runtime/timestamp-parse-util.h: http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.h@190 PS6, Line 190: // str is to point to the opening quote when the function is called. Once the function > Let me reflect it on the next patch set. Done http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.cc File be/src/runtime/timestamp-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.cc@174 PS6, Line 174: length_of_string_literal = str - quoted_string_begin - 1; > Right, it should be a redundant. Let me remove this on the next patch set. Done http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.cc@176 PS6, Line 176: // e.g. from_unixtime(0, '\'\'') => ' > Duplicate. Let me remove this on the next patch set. Done http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.cc@177 PS6, Line 177: if (length_of_string_literal == 0) length_of_string_literal = 1; > Okay, let me reflect this on the next patch set. Done http://gerrit.cloudera.org:8080/#/c/8508/6/be/src/runtime/timestamp-parse-util.cc@178 PS6, Line 178: position_of_string_literal = str - str_begin - length_of_string_literal; > Okay, I accept your approach. Done -- 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: 7 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: Fri, 26 Jan 2018 06:09:56 +0000 Gerrit-HasComments: Yes
