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 http://gerrit.cloudera.org:8080/5951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <zams...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-HasComments: No
