> 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?
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. - Ashish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22612/#review46008 ----------------------------------------------------------- On June 16, 2014, 2:06 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22612/ > ----------------------------------------------------------- > > (Updated June 16, 2014, 2:06 a.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 > 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 > >
