hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Dere <jd...@hortonworks.com>
Subject Re: Review Request 58934: HIVE-16568: Support complex types in external LLAP InputFormat
Date Fri, 05 May 2017 02:48:28 GMT


> On May 3, 2017, 5:34 a.m., Prasanth_J wrote:
> > llap-client/src/java/org/apache/hadoop/hive/llap/LlapRowRecordReader.java
> > Lines 154 (patched)
> > <https://reviews.apache.org/r/58934/diff/1/?file=1706163#file1706163line174>
> >
> >     IIRC, there are utilities already to do this in ObjectInspectorUtils.. copyToXXX()
methods.. can that be reused?
> 
> Jason Dere wrote:
>     I had not seen the methods in ObjectInspectorUtils, yeah we might be able to use
this.

copyToXXX() ends up saving decimal/char/varchar values to their Hive equivalents (HiveDecimal/HiveChar/HiveVarchar).
I'd prefer the row interface expose these values as non-Hive classes (BigDecimal/String/String)
for ease of use. So I think it is better off not using the ObjectInspecterUtils.copyToXXX
methods.


> On May 3, 2017, 5:34 a.m., Prasanth_J wrote:
> > llap-common/src/java/org/apache/hadoop/hive/llap/TypeDesc.java
> > Lines 154 (patched)
> > <https://reviews.apache.org/r/58934/diff/1/?file=1706165#file1706165line154>
> >
> >     This also looks repetitive. TypeInfoUtils already has something like this I
guess. We need to make sure TypeInfo parser can parse the string generated by this method.
It's easier to reuse TypeInfoUtils or have a TypeDesc converted to TypeInfo.
> >     
> >     My point there is duplicacy in
> >     TypeInfo
> >     TypeDesc
> >     TypeDescriptor (ORC has this)
> >     
> >     Wondering if TypeInfo or TypeDescriptor from ORC can be reused here. Thoughts?
> 
> Jason Dere wrote:
>     I had orginally created TypeDesc to try to avoid extra dependencies on what might
be considered internal Hive types, but it is true that we are still the serde lib and TypeInfo
is available from there. At this point though the TypeDesc has already been created, I'm not
totally sure if I want to now go and remove this. If you feel strongly about this let me know.
> 
> Prasanth_J wrote:
>     May be this can be done in a follow up if it involves a lot of surgery.
>     If removing hive dependency is desired or required, then there are 2 possibilities
(Make TypeDescriptor in ORC a separate module or make TypeInfo in hive as separate module).
But at the current state it looks like there is dependecy with hive serde and typeinfo, so
it might be easier to reuse code.
>     This way there won't be problem with maintaining compatibility with hive types when
new one gets added.

I've tried to update the patch to remove TypeDesc and use TypeInfo instead.


- Jason


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


On May 2, 2017, 9:52 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58934/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 9:52 p.m.)
> 
> 
> Review request for hive, Gunther Hagleitner, Prasanth_J, and Siddharth Seth.
> 
> 
> Bugs: HIVE-16568
>     https://issues.apache.org/jira/browse/HIVE-16568
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> - Support list/map/struct types in the LLAPRowInputFormat Schema/TypeDesc
> - Support list/map/struct types in the LLAPRowInputFormat Row. Changes in the Row getters/setters
needed (no longer using Writable).
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlap.java de47412

>   llap-client/src/java/org/apache/hadoop/hive/llap/LlapRowRecordReader.java ee92f3e 
>   llap-common/src/java/org/apache/hadoop/hive/llap/Row.java a84fadc 
>   llap-common/src/java/org/apache/hadoop/hive/llap/TypeDesc.java dda5928 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 9ddbd7e

> 
> 
> Diff: https://reviews.apache.org/r/58934/diff/1/
> 
> 
> Testing
> -------
> 
> Added test to TestJdbcWithMiniLlap
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message