> On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote: > > common/pom.xml, lines 59-63 > > <https://reviews.apache.org/r/29898/diff/2/?file=825966#file825966line59> > > > > Since joda jar will be shipped to task nodes, this needs to be added in > > hive-exec jar. I think we keep that list in one of the pom files. We need > > to add this dep there. > > Jason Dere wrote: > Do you mean the artifact set for the shaded JAR goal in ql/pom.xml? I'll > take a look at doing this.
yeah.. essentially it needs to be hive-exec jar (which is shaded these days). Else, while running on cluster, this will fail with CNF exception. Once you do this, it will be good to test this on cluster to confirm that we got this right. > On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote: > > common/src/java/org/apache/hive/common/util/TimestampParser.java, line 76 > > <https://reviews.apache.org/r/29898/diff/2/?file=825967#file825967line76> > > > > Name suggests this can be an instance object. If we do that way, than > > we can avoid creating this object per invocation, which will be nice if > > possible. > > Jason Dere wrote: > The way this is currently set up the LazyTimestampObjectInspector (which > I believe could be shared by different threads) points to a single > TimestampParser. The Joda DateTimeFormatter is thread safe, so everything in > parseTimestamp() should be thread safe except for mdt which is why I was > creating a new object. I guess mdt could be made thread safe by making it a > thread-local instance. I'll make that change. I see. Didn't realize mdt isnt thread safe. I think in that case, you can leave this at is, because I am not sure accessing a thread local variable is cheaper than creating new() object. I hear these days, new() is much faster than many other things within jvm and there is no need to optimize it unless proven. So, I will suggest this to leave this as it is for now. > On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote: > > common/src/java/org/apache/hive/common/util/TimestampParser.java, line 127 > > <https://reviews.apache.org/r/29898/diff/2/?file=825967#file825967line127> > > > > Can't we do Long.valueOf()? That will be faster than BD parsing, I > > presume. > > Jason Dere wrote: > If we don't want to worry about fractional millisecond values, then we > can do this. We're throwing away the fractional portion anyway since Joda > does not have precision less than 1 ms. I'll change this. Sounds good. > On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote: > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java, > > line 649 > > <https://reviews.apache.org/r/29898/diff/2/?file=825983#file825983line649> > > > > I think there is a helper method in apache commons (or guava) which can > > let you do such parsing. Will be good to reuse that, if available. > > Jason Dere wrote: > Not sure if the commons/guava libs have something to escape commas > (please correct me if I am wrong). I see that Hive uses opencsv which handles > CSV-style escaping, I will use this to parse the list. Other possibility is to potentially refactor utility method from HIVE-4576 for this. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29898/#review69930 ----------------------------------------------------------- On Jan. 20, 2015, 12:34 a.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29898/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2015, 12:34 a.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-9298 > https://issues.apache.org/jira/browse/HIVE-9298 > > > Repository: hive-git > > > Description > ------- > > Add new SerDe parameter "timestamp.formats" to specify alternate timestamp > patterns > > > Diffs > ----- > > common/pom.xml ede8aea > common/src/java/org/apache/hive/common/util/TimestampParser.java > PRE-CREATION > common/src/test/org/apache/hive/common/util/TestTimestampParser.java > PRE-CREATION > data/files/ts_formats.txt PRE-CREATION > > hbase-handler/src/java/org/apache/hadoop/hive/hbase/DefaultHBaseKeyFactory.java > 98bc73f > > hbase-handler/src/java/org/apache/hadoop/hive/hbase/HBaseLazyObjectFactory.java > 78f23cb > > hbase-handler/src/java/org/apache/hadoop/hive/hbase/struct/AvroHBaseValueFactory.java > a2ba827 > > hbase-handler/src/java/org/apache/hadoop/hive/hbase/struct/DefaultHBaseValueFactory.java > e60b844 > hbase-handler/src/test/queries/positive/hbase_timestamp_format.q > PRE-CREATION > hbase-handler/src/test/results/positive/hbase_timestamp_format.q.out > PRE-CREATION > pom.xml c147d45 > ql/src/test/queries/clientpositive/timestamp_formats.q PRE-CREATION > ql/src/test/results/clientpositive/timestamp_formats.q.out PRE-CREATION > > serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java > 8d3595b > > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroLazyObjectInspector.java > 2fb1c28 > serde/src/java/org/apache/hadoop/hive/serde2/columnar/ColumnarSerDe.java > 882c43e > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyFactory.java e3968a9 > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java > 95e30db > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyTimestamp.java > 27895c5 > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazyUtils.java 3943508 > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/LazyListObjectInspector.java > 9d66a78 > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/LazyMapObjectInspector.java > ee870f5 > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/LazyObjectInspectorFactory.java > 1abd8a5 > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/LazySimpleStructObjectInspector.java > 9611e9f > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/LazyUnionObjectInspector.java > 792a9a2 > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyObjectInspectorParameters.java > PRE-CREATION > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyObjectInspectorParametersImpl.java > PRE-CREATION > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyPrimitiveObjectInspectorFactory.java > 08fec77 > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/objectinspector/primitive/LazyTimestampObjectInspector.java > 0d15054 > > Diff: https://reviews.apache.org/r/29898/diff/ > > > Testing > ------- > > Added CliDriver/HBaseCliDriver qfile tests > > > Thanks, > > Jason Dere > >
