Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/12267 )
Change subject: IMPALA-4018 Part1: Add FORMAT clause in CAST() ...................................................................... Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/12267/5/be/src/exprs/cast-expr.h File be/src/exprs/cast-expr.h: http://gerrit.cloudera.org:8080/#/c/12267/5/be/src/exprs/cast-expr.h@43 PS5, Line 43: mutable std::unique_ptr<datetime_parse_util::DateTimeFormatContext> dt_ctx > Smart pointer seemed a more error proof: This way one won't forget to do a I see, I forgot about the error path. Please mention in the comment that this is can be empty if OpenEvaluator was called with THREAD_LOCAL scope. Or it could be handled as THREAD_LOCAL, and in that case ctx->GetFunctionState() would not be needed at all, as every CastExpr could fill its dt_ctx in OpenEvaluator() and use it during casting. + nit: should be dt_ctx_ as it is a class member -- To view, visit http://gerrit.cloudera.org:8080/12267 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia514aaa9e8f5487d396587d5ed24c7348a492697 Gerrit-Change-Number: 12267 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Greg Rahn <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Paul Rogers <[email protected]> Gerrit-Comment-Date: Wed, 13 Feb 2019 12:33:27 +0000 Gerrit-HasComments: Yes
