----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11748/#review21666 -----------------------------------------------------------
Ship it! LGTM. It looks like java identifiers will also be prepended with _ now but that should not affect the consumption of the schema in other scenarios. - Venkat Ranganathan On June 8, 2013, 3:56 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11748/ > ----------------------------------------------------------- > > (Updated June 8, 2013, 3:56 p.m.) > > > Review request for Sqoop. > > > Description > ------- > > The issue is hiding in fact that ClassWriter will use toJavaIdentifier() > method to generate attribute name, however AvroSchemaGenerator is using only > toIdentifier(). I've changed the AvroSchemaGenerator to generate the proper > schema in the same way as is doing the ClassWriter. This is backward > incompatible change, however considering that Sqoop was not able to import > anything without this, I would assume that consequences will be minimal if > there will be any. > > > This addresses bug SQOOP-1071. > https://issues.apache.org/jira/browse/SQOOP-1071 > > > Diffs > ----- > > src/java/org/apache/sqoop/orm/AvroSchemaGenerator.java > 864fa0d83c981043e3ff75854d8575ab13e00296 > src/test/com/cloudera/sqoop/TestAvroImport.java > 32d1bbea7459ce6f28253d74571e786749030cf0 > src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java > cf41b9691fa3c18c7caac99f27ebf327687a0649 > > Diff: https://reviews.apache.org/r/11748/diff/ > > > Testing > ------- > > I've added test that will fail without the fix and refactored a bit > BaseSqoopTestCase to allow override column names. All tests seems to be > passing. > > > Thanks, > > Jarek Cecho > >
