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 Fri, 22 Sep 2017 13:35:26 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:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/exprs/timestamp-functions-ir.cc@332
PS2, Line 332:   DCHECK(ts_value.IsValidDate());
> why does this DCHECK no longer make sense?  are we saying it's trivially tr
Yes, my idea was that range check must be done at construction, and it is enough to check
HasDate() in other timestamp related functions.



-- 
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: Fri, 22 Sep 2017 13:35:26 +0000
Gerrit-HasComments: Yes

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