Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201710022151.v92Lp5Io017098@ip-10-146-233-104.ec2.internal>
Date: Mon, 2 Oct 2017 21:51:05 +0000
From: "Dan Hecht (Code Review)" <gerrit@cloudera.org>
To: Csaba Ringhofer <csringhofer@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Lars Volker <lv@cloudera.com>
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-5664=3A_Unix_time_to_timestamp_conversions_may_crash_Impala=0A?=
In-Reply-To: <gerrit.1504538493000.I77b2f6284d3a597f57e61c17a67c959eff9e38ff@gerrit.cloudera.org>
References: <gerrit.1504538493000.I77b2f6284d3a597f57e61c17a67c959eff9e38ff@gerrit.cloudera.org>
Reply-To: dhecht@cloudera.com, impala-cr@cloudera.com, lv@cloudera.com,
        marcelk@gmail.com, csringhofer@cloudera.com,
        reviews@impala.incubator.apache.org
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="tFJUy5K0cqM="; charset=UTF-8
archived-at: Mon, 02 Oct 2017 21:51:13 -0000

--tFJUy5K0cqM=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Dan Hecht has posted comments on this change=2E ( http://gerrit=2Ecloudera=
=2Eorg:8080/7954 )

Change subject: IMPALA-5664: Unix time to timestamp con=
versions may crash Impala
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E


Patch Set 9:

(4 comments)

http://gerrit=2Ecloudera=2Eorg:8=
080/#/c/7954/9/be/src/exprs/timestamp-functions-ir=2Ecc
File be/src/exprs/t=
imestamp-functions-ir=2Ecc:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/7954/9=
/be/src/exprs/timestamp-functions-ir=2Ecc@136
PS9, Line 136: if(!tv=2EHasDa=
teAndTime()){
here an elsewhere - please follow the style used by the rest =
of impala=2E


http://gerrit=2Ecloudera=2Eorg:8080/#/c/7954/9/be/src/exprs/=
timestamp-functions-ir=2Ecc@137
PS9, Line 137: UnixMicrosToUtcTimestamp
is =
that meaningful to the user?


http://gerrit=2Ecloudera=2Eorg:8080/#/c/7954=
/9/be/src/exprs/timestamp-functions-ir=2Ecc@139
PS9, Line 139:   }
are the =
ones you added the only places where your new validation might produce a !H=
asDateAndTime() TimestampValue?


http://gerrit=2Ecloudera=2Eorg:8080/#/c/7=
954/9/be/src/exprs/timestamp-functions-ir=2Ecc@751
PS9, Line 751:     datet=
ime=2Edate()=2Eyear();
it still seems like this is doing the same validatio=
n=2E should we remove this an instead do the result_value=2EHasDateAndTime(=
) check here? Or is this really a different case?


-- 
To view, visit htt=
p://gerrit=2Ecloudera=2Eorg:8080/7954
To unsubscribe, visit http://gerrit=
=2Ecloudera=2Eorg:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: =
master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61=
c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 9
Gerrit-Ow=
ner: Csaba Ringhofer <csringhofer@cloudera=2Ecom>
Gerrit-Reviewer: Csaba Ri=
nghofer <csringhofer@cloudera=2Ecom>
Gerrit-Reviewer: Dan Hecht <dhecht@clo=
udera=2Ecom>
Gerrit-Reviewer: Lars Volker <lv@cloudera=2Ecom>
Gerrit-Commen=
t-Date: Mon, 02 Oct 2017 21:51:05 +0000
Gerrit-HasComments: Yes

--tFJUy5K0cqM=--