Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Hive, mail # dev - Re: Review Request 11925: Hive-3159 Update AvroSerde to determine schema of new tables


Copy link to this message
-
Re: Review Request 11925: Hive-3159 Update AvroSerde to determine schema of new tables
Mohammad Islam 2013-07-23, 01:56


> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java, line 67
> > <https://reviews.apache.org/r/11925/diff/3/?file=320914#file320914line67>
> >
> >     Should be a debug

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java, line 77
> > <https://reviews.apache.org/r/11925/diff/3/?file=320914#file320914line77>
> >
> >     spacing

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java, line 125
> > <https://reviews.apache.org/r/11925/diff/3/?file=320914#file320914line125>
> >
> >     Use j here instead, so as not to shadow the i.

Not clear. What is it shadowing? There is no other 'i'.
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 36
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line36>
> >
> >     Some spacing on the table would help.

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 77
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line77>
> >
> >     formatting

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 93
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line93>
> >
> >     fot -> for.  type info -> TypeInfo

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 98
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line98>
> >
> >     Is there any way to turn off the union wrapping?  If not, does it require its own separate parameter?

'wrapUnion' flag is needed to avoid the duplicated "Union". Union schema does not required to wrap with union. I think it is an exception, if it does.
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 100
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line100>
> >
> >     formatting

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 109
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line109>
> >
> >     Move this above the first function so it's clear to readers.

Done.
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 111
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line111>
> >
> >     Yeah, do that.

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 125
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line125>
> >
> >     Doesn't this duplicate generateSchema above?

Removed
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 211
> > <https://reviews.apache.org/r/11925/diff/3/?file=320915#file320915line211>
> >
> >     can this be private?

It is used by Test case.
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java, line 56
> > <https://reviews.apache.org/r/11925/diff/3/?file=320917#file320917line56>
> >
> >     delete

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java, line 121
> > <https://reviews.apache.org/r/11925/diff/3/?file=320917#file320917line121>
> >
> >     delete

Done
> On July 12, 2013, 10:32 p.m., Jakob Homan wrote:
> > serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java, line 127

Done

Done. Added msg.

Done. content checked .

Done

Done

Done

Done

Normal CTAS also does not copy the comments.

Done

This test has the "group by".

There is a hive problem of creating internal column names such as  col0, col1 that creates CTAS command to fail. This one needs to be resolved in a separate JIRA.
- Mohammad
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11925/#review23102
On July 12, 2013, 6:49 p.m., Mohammad Islam wrote: