> 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.
Do you mean the artifact set for the shaded JAR goal in ql/pom.xml? I'll take a look at doing this. > 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. 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. > 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. 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. > On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote: > > serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java, > > line 114 > > <https://reviews.apache.org/r/29898/diff/2/?file=825979#file825979line114> > > > > This is thrift generated file. Instead of hand modifying you need to > > put this in thrift file and generate it via thrift compiler. Whoops missed that, thanks for pointing that out, will fix. > On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote: > > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java, > > lines 135-137 > > <https://reviews.apache.org/r/29898/diff/2/?file=825983#file825983line135> > > > > I wonder why these and lastColtakeRest are not included in > > LazyOIParams. Seems to me, they should be included too. If you think > > otherwise, it will be good to add a comment here about what distinguishes > > these two set of params. So I thought these params had more to do with the SerDe and handling of rows than they did with actual values and ObjectInspector-related handling, which is why I left those out of the lazy OI params. Admittedly it does look a bit odd to bundle some of the params together and leave others out. If you think I should just include those in I can do so. > 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. 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. - Jason ----------------------------------------------------------- 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 > >
