impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Csaba Ringhofer (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala
Date Thu, 21 Sep 2017 17:29:05 GMT
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/7954
)

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7954/5/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/5/be/src/runtime/timestamp-value.h@173
PS5, Line 173: 
This became static to enable other functions to check whether their date is valid or not without
actually creating a TimestampValue or calling year() to generate an exception. I think it
would be better if there was a single function that does this check, so if 1400 will be changed
for some reason ( IMPALA-2009 ), there will be no need to change many parts of the code.


http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-value.h@86
PS2, Line 86:     }
> One purpose of FunctionContext is to be the interface back into impala's ru
I think it would be better to do the logging on the caller side, because depending on the
caller, the format for the log can be different, e.g. unix timestamp vs YYYY-MM-DD string.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <csringhofer@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringhofer@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:29:05 +0000
Gerrit-HasComments: Yes

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