Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-31 Thread Ashutosh Chauhan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29898/#review70512
---

Ship it!


Ship It!

- Ashutosh Chauhan


On Jan. 31, 2015, 2:25 a.m., Jason Dere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29898/
> ---
> 
> (Updated Jan. 31, 2015, 2:25 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 ccd4c2c 
>   common/src/java/org/apache/hive/common/util/HiveStringUtils.java 8db2c2c 
>   common/src/java/org/apache/hive/common/util/TimestampParser.java 
> PRE-CREATION 
>   common/src/test/org/apache/hive/common/util/TestHiveStringUtils.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 ecec5af 
>   ql/pom.xml 87f79e2 
>   ql/src/test/queries/clientpositive/timestamp_formats.q PRE-CREATION 
>   ql/src/test/results/clientpositive/timestamp_formats.q.out PRE-CREATION 
>   serde/if/serde.thrift 76f95b5 
>   serde/src/gen/thrift/gen-cpp/serde_constants.h 1455382 
>   serde/src/gen/thrift/gen-cpp/serde_constants.cpp bd5c16d 
>   
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java
>  8d3595b 
>   
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/test/ThriftTestObj.java
>  1b708dd 
>   
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/Complex.java
>  07ea8b9 
>   
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/MegaStruct.java
>  386fef9 
>   
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/PropValueUnion.java
>  aa56dc9 
>   
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/SetIntString.java
>  676f2b2 
>   serde/src/gen/thrift/gen-php/org/apache/hadoop/hive/serde/Types.php 3c2f0a9 
>   serde/src/gen/thrift/gen-py/org_apache_hadoop_hive_serde/constants.py 
> 08ca294 
>   serde/src/gen/thrift/gen-rb/serde_constants.rb 40375f5 
>   
> 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/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
> 
>

Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-30 Thread Jason Dere 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29898/
---

(Updated Jan. 31, 2015, 2:25 a.m.)


Review request for hive and Ashutosh Chauhan.


Changes
---

changes per RB feedback:
- Add Joda to shade goal
- Use Long.parseLong() rather than BigDecimal
- change serdeConstants using thrift file
- Add separators/nullSequence/lastColumnTakesRest to lazy OI params, cuts down 
on the number of args in the various lazy methods.
- Change comma-delimited parsing


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 (updated)
-

  common/pom.xml ccd4c2c 
  common/src/java/org/apache/hive/common/util/HiveStringUtils.java 8db2c2c 
  common/src/java/org/apache/hive/common/util/TimestampParser.java PRE-CREATION 
  common/src/test/org/apache/hive/common/util/TestHiveStringUtils.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 ecec5af 
  ql/pom.xml 87f79e2 
  ql/src/test/queries/clientpositive/timestamp_formats.q PRE-CREATION 
  ql/src/test/results/clientpositive/timestamp_formats.q.out PRE-CREATION 
  serde/if/serde.thrift 76f95b5 
  serde/src/gen/thrift/gen-cpp/serde_constants.h 1455382 
  serde/src/gen/thrift/gen-cpp/serde_constants.cpp bd5c16d 
  
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java
 8d3595b 
  
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/test/ThriftTestObj.java
 1b708dd 
  
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/Complex.java
 07ea8b9 
  
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/MegaStruct.java
 386fef9 
  
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/PropValueUnion.java
 aa56dc9 
  
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/SetIntString.java
 676f2b2 
  serde/src/gen/thrift/gen-php/org/apache/hadoop/hive/serde/Types.php 3c2f0a9 
  serde/src/gen/thrift/gen-py/org_apache_hadoop_hive_serde/constants.py 08ca294 
  serde/src/gen/thrift/gen-rb/serde_constants.rb 40375f5 
  
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/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

Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-30 Thread Jason Dere 


> On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java, 
> > line 649
> > 
> >
> > 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.
> 
> Ashutosh Chauhan wrote:
> Other possibility is to potentially refactor utility method from 
> HIVE-4576 for this.

TestTempletonUtils.unEscapeString() is actually buggy due to the attempt to 
only unescape '\,' and nothing else. The following string would cause an 
error:"paths=c:\path1\,c:\path2". You need to escape '\' as '\' or 
StringUtils.unEscapeString() will throw an exception.
I've created a general version in HiveStringUtils which does what my ugly 
custom parser did, but I have not changed the logic in HIVE-4576 due to 
potential backward compatibility issues.


- 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
> 
>

Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-28 Thread Ashutosh Chauhan 


> On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote:
> > common/pom.xml, lines 59-63
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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

Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-28 Thread Jason Dere 


> On Jan. 28, 2015, 1:22 a.m., Ashutosh Chauhan wrote:
> > common/pom.xml, lines 59-63
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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/

Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-27 Thread Ashutosh Chauhan 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29898/#review69930
---



common/pom.xml


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.



common/src/java/org/apache/hive/common/util/TimestampParser.java


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.



common/src/java/org/apache/hive/common/util/TimestampParser.java


Can't we do Long.valueOf()? That will be faster than BD parsing, I presume.



serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java


This is thrift generated file. Instead of hand modifying you need to put 
this in thrift file and generate it via thrift compiler.



serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java


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.



serde/src/java/org/apache/hadoop/hive/serde2/lazy/LazySimpleSerDe.java


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.


- Ashutosh Chauhan


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/

Re: Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-19 Thread Jason Dere 

---
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.


Changes
---

Failures in unit test were because lazy map object inspector cache was adding 
entire lazyParams object to signature, rather than adding the individual 
fields. Attaching patch v2.


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 (updated)
-

  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

Review Request 29898: HIVE-9298: Support reading alternate timestamp formats

2015-01-14 Thread Jason Dere 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29898/
---

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/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 416bc7a 
  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 
1587be8 
  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