impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Date Mon, 27 Nov 2017 14:49:55 GMT
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 )

Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME()
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

Leaving a +1 in case Dan has any more comments before he +2s.

http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@82
PS2, Line 82:  
> Thanks for the information. I guess TimestampValue might have a dangling po
I don't remember which which commit id or gerrit link.

I don't think this will create any CPU cost, due to the return-value optimization, which was
common even years ago:

http://en.cppreference.com/w/cpp/language/copy_elision


http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@96
PS2, Line 96: &
> Is it a problematic code also?
This is also not ideal, but probably not worth fixing in this commit, since commits should
mostly address a single thing.



-- 
To view, visit http://gerrit.cloudera.org:8080/8629
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2
Gerrit-Change-Number: 8629
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul <jinchul@gmail.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Kim Jin Chul <jinchul@gmail.com>
Gerrit-Comment-Date: Mon, 27 Nov 2017 14:49:55 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message