Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14291 )
Change subject: IMPALA-8704: ISO:SQL:2016 datetime patterns - Milestone 2 ...................................................................... Patch Set 15: (3 comments) http://gerrit.cloudera.org:8080/#/c/14291/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14291/14//COMMIT_MSG@28 PS14, Line 28: This is the default behaviour > What does "default behavior" mean in this context? Is it when no FX or FM m Default is when neither FX or FM are given. http://gerrit.cloudera.org:8080/#/c/14291/14/be/src/runtime/datetime-iso-sql-format-tokenizer.cc File be/src/runtime/datetime-iso-sql-format-tokenizer.cc: http://gerrit.cloudera.org:8080/#/c/14291/14/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@115 PS14, Line 115: fm_modifier_active_ = fa > nit: not necessary, you can just set the flag to false no matter what. Done http://gerrit.cloudera.org:8080/#/c/14291/14/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@237 PS14, Line 237: DCHECK(*current_pos == dt_ctx_->fmt); > It would be more straightforward if ProcessFXModifier() did not take any pa I have to pass **current_pos into this function as a param because the function is expected to advance the pointer after the FX modifier. -- To view, visit http://gerrit.cloudera.org:8080/14291 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d2f6656054371476aaa8bd0d51f572b9369855 Gerrit-Change-Number: 14291 Gerrit-PatchSet: 15 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, 06 Nov 2019 11:48:42 +0000 Gerrit-HasComments: Yes
