> On June 17, 2014, 11:01 p.m., Swarnim Kulkarni wrote: > > common/src/java/org/apache/hadoop/hive/common/type/HiveDecimal.java, line 55 > > <https://reviews.apache.org/r/22612/diff/1/?file=610526#file610526line55> > > > > This doesn't seem specific to the HiveDecimal class. Why not simply > > make it private and move to the class where it is actually being used? > > Ashish Singh wrote: > As of now its used just at one place, so it kind of seems OK to move it > close to its usage. However, this might not be the only place where it is > required. Moreover, having nullBytes in DT def class is practiced at other > places in Hive, like, TimestampWritable. I am inclined towards having it as > it is now. Let me know if you strongly disagree.
I am more in favor of "When in doubt, leave it out!" :). My vote would be to move it to where it is being used currently and then refactor out to a common location if needed in future. - Swarnim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22612/#review46008 ----------------------------------------------------------- On June 18, 2014, 7:11 p.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22612/ > ----------------------------------------------------------- > > (Updated June 18, 2014, 7:11 p.m.) > > > Review request for hive, Szehon Ho and Xuefu Zhang. > > > Bugs: HIVE-7234 > https://issues.apache.org/jira/browse/HIVE-7234 > > > Repository: hive-git > > > Description > ------- > > HIVE-7234: Handle nulls from decimal columns elegantly > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/type/HiveDecimal.java > ad0901548217fbb828a01f8f5edda64581ac2c1e > data/files/decimal_10_0.txt PRE-CREATION > data/files/decimal_9_0.txt PRE-CREATION > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestDecimal.java > PRE-CREATION > ql/src/test/queries/clientpositive/decimal_10_0.q PRE-CREATION > ql/src/test/results/clientpositive/decimal_10_0.q.out PRE-CREATION > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyHiveDecimal.java > 78cc3819c61f5a1bcb0cdd3425a0105416c26861 > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyUtils.java > 5a4623729ec955bbe8fcf662503b42ff8735eaad > > Diff: https://reviews.apache.org/r/22612/diff/ > > > Testing > ------- > > Added unit tests to test the scenario. > > > Thanks, > > Ashish Singh > >
