impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-2020: Add rounding when casting from decimal to int
Date Thu, 09 Feb 2017 19:42:05 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-2020: Add rounding when casting from decimal to int

Patch Set 3:

> > I'm not opposed to cleaning up the AnyVal stuff like that, but
 > > given that udf.h stuff dictates UDF compatibility, it's not
 > > completely trivial. It doesn't look like it would break binary
 > > compatibility though. But, in case something goes wrong, how
 > about
 > > we do that as a separate change so it could be backed out without
 > > affecting the decimal work? It doesn't look like the decimal
 > stuff
 > > will depend on it, right?
 > No, but it gets a lot cleaner to test the limits by giving the
 > generic form an underlying type.  I deliberately did not change
 > FloatVal, since the equality operator is currently kind of broken. 
 > Everything else should be binary compatible.

But you can get that by looking at *Val::val, no?

Another thing to watch out for is that since users write and compile their own UDFs, we've
had trouble in the past with accidently increasing the C++ version required to build these
things. I don't remember the specific problems (maybe Michael or Tim do) and also don't know
whether that's a real issue with this change.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Zach Amsden <>
Gerrit-HasComments: No

View raw message