Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14714 )
Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3 ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/14714/3/be/src/runtime/datetime-parser-common.cc File be/src/runtime/datetime-parser-common.cc: http://gerrit.cloudera.org:8080/#/c/14714/3/be/src/runtime/datetime-parser-common.cc@210 PS3, Line 210: DCHECK(tok.type == MONTH_NAME); : bool is_token_padded = fx_modifier && !tok.fm_modifier; : bool following_separator_found = token_start + token_len != input_end && : IsoSqlFormatTokenizer::IsSeparator(*(token_start + token_len)); : if (is_token_padded || following_separator_found) { : DCHECK(token_len == MAX_MONTH_NAME_LENGTH || !is_token_padded); : if (is_token_padded) trim_right_if(buff, boost::algorithm::is_any_of(" ")); : const auto iter = MONTH_NAME_TO_INDEX.find(buff); : if (UNLIKELY(iter == MONTH_NAME_TO_INDEX.end())) return false; : *month = iter->second; : return true; : } If you implement the optimization suggested below, this block probably wouldn't be needed at all. http://gerrit.cloudera.org:8080/#/c/14714/3/be/src/runtime/datetime-parser-common.cc@329 PS3, Line 329: DCHECK(tok.len >= SHORT_DAY_NAME_LENGTH); Add: && tok.len >= SHORT_MONTH_NAME_LENGTH condition. http://gerrit.cloudera.org:8080/#/c/14714/3/be/src/runtime/timestamp-parse-util.cc File be/src/runtime/timestamp-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/14714/3/be/src/runtime/timestamp-parse-util.cc@255 PS3, Line 255: case DAY_OF_WEEK: { Please add a comment explaining that 1 means Sunday, 2 means Monday, and so on. -- To view, visit http://gerrit.cloudera.org:8080/14714 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f Gerrit-Change-Number: 14714 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 20 Nov 2019 09:44:56 +0000 Gerrit-HasComments: Yes
