----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7042/#review11358 -----------------------------------------------------------
Overall this looks good. The actual change is quite small, most of this is testing. My guess is all tests will fail due to the issue below and I think it might be easier to fix for the reason in that comment. serde/if/test/complex.thrift <https://reviews.apache.org/r/7042/#comment24405> How about we move this empty struct definition to MegaStruct? That's the place to stress-test thrift support. Its also likely to be a smaller change, because any tests that have expected Complex output will need updated, and there are more than tests using MegaStruct. http://svn.apache.org/viewvc/hive/trunk/serde/if/test/megastruct.thrift?view=markup - Travis Crawford On Sept. 11, 2012, 9:19 p.m., Feng Peng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7042/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2012, 9:19 p.m.) > > > Review request for hive, Ashutosh Chauhan, Dmitriy Ryaboy, and Travis > Crawford. > > > Description > ------- > > Right now TypeInfoUtils expects at least one field in a STRUCT, which is not > always true, e.g., empty struct is allowed in Thrift. Modified TypeInfoUtils > so that empty struct can be correctly processed. > > > This addresses bug HIVE-3398. > https://issues.apache.org/jira/browse/HIVE-3398 > > > Diffs > ----- > > serde/if/test/complex.thrift 308b64c > > serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/Complex.java > f5a2986 > > serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde2/thrift/test/EmptyStruct.java > PRE-CREATION > serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java > 6c07ab5 > > serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestThriftObjectInspectors.java > 5f692fb > > serde/src/test/org/apache/hadoop/hive/serde2/thrift_test/CreateSequenceFile.java > 8aef773 > > Diff: https://reviews.apache.org/r/7042/diff/ > > > Testing > ------- > > > Thanks, > > Feng Peng > >
