> On Nov. 12, 2014, 7:46 p.m., Jarek Cecho wrote: > > common/src/main/java/org/apache/sqoop/schema/type/AbstractComplexType.java, > > line 25 > > <https://reviews.apache.org/r/27918/diff/1/?file=760208#file760208line25> > > > > Could you talk a bit about what does the size represents here? (and > > prehaps include a comment describing that) > > Veena Basavaraj wrote: > good point, I was wondering about this myself yday and did not get a > chance to clarify. > > related to my question. On the string we have a length. is this basically > a max lenght? that we will use for validation ? > > I added size on the similar lines, may be all we care is the byte size, > just like the number has.
I would probably add size only to those types where it make sense. Char/Varchar do have size, numbers do have size as well. Arrays doesn't seem to have size (in Hive, and bogus one in PostgresSQL), Map and Enum doesn't have size either. I did nice summary and investigation in SQOOP-515, perhaps it might be useful to crosscheck my notes. - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27918/#review61043 ----------------------------------------------------------- On Nov. 12, 2014, 7:46 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27918/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2014, 7:46 p.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Bugs: SQOOP-1709 > https://issues.apache.org/jira/browse/SQOOP-1709 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see jira > > NOTE: WS will be addressed once the patch is reviewd for functinality > > Have a few questions as part of this patch: > > Why do we make the name optional? It actually throws an exception if we have > more than one column as empty. So would it not be wise to make it a > mandatory? or can we have columns with no names in databases? > > Second, the length attribute, is it more like maxLength that the columns > have? if so we should just call it so. > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java > ecb5aa3 > > common/src/main/java/org/apache/sqoop/schema/type/AbstractComplexListType.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/AbstractComplexType.java > ca9b3b1 > common/src/main/java/org/apache/sqoop/schema/type/AbstractDateTime.java > 5c969af > common/src/main/java/org/apache/sqoop/schema/type/AbstractNumber.java > 1ecb2d5 > > common/src/main/java/org/apache/sqoop/schema/type/AbstractPrimitiveType.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/AbstractString.java > 950c2fd > common/src/main/java/org/apache/sqoop/schema/type/Array.java d7dcc0c > common/src/main/java/org/apache/sqoop/schema/type/Binary.java 7111bf5 > common/src/main/java/org/apache/sqoop/schema/type/Bit.java 2371746 > common/src/main/java/org/apache/sqoop/schema/type/Column.java 2339d66 > common/src/main/java/org/apache/sqoop/schema/type/Date.java 324a089 > common/src/main/java/org/apache/sqoop/schema/type/DateTime.java d982d2a > common/src/main/java/org/apache/sqoop/schema/type/Decimal.java 5470b9c > common/src/main/java/org/apache/sqoop/schema/type/Enum.java 0c54fd0 > common/src/main/java/org/apache/sqoop/schema/type/FixedPoint.java d949aa7 > common/src/main/java/org/apache/sqoop/schema/type/FloatingPoint.java > 9639197 > common/src/main/java/org/apache/sqoop/schema/type/Map.java d3301c0 > common/src/main/java/org/apache/sqoop/schema/type/Set.java a18699f > common/src/main/java/org/apache/sqoop/schema/type/Text.java db47ede > common/src/main/java/org/apache/sqoop/schema/type/Time.java 0f71508 > common/src/main/java/org/apache/sqoop/schema/type/Unknown.java 97f72c0 > > common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java > 1640ead > > Diff: https://reviews.apache.org/r/27918/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
