----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69060/ -----------------------------------------------------------
(Updated Nov. 8, 2018, 3:34 p.m.) Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas. Changes ------- The most notable change in this newest changesetis around verification: - as we discussed with Szabi: since decimal conversion is put in place it's not enough to assert on the data itself (as the data appears as String in-memory, it could be String on the disk as well) - so, I've added a new function that checks the schema for decimal types as well Bugs: SQOOP-3382 https://issues.apache.org/jira/browse/SQOOP-3382 Repository: sqoop-trunk Description ------- This patch is about adding support for fixed point decimal types in parquet import. The implementation is simple after the fact that parquet was upgraded to 1.9.0 in SQOOP-3381: we just need to register the GenericDataSupplier with AvroParquetOutputFormat. For testing, we can reuse the existing Avro tests, because Sqoop uses Avro under the hood to write parquet. I also moved around and renamed the classes involved in this change so their name and package reflect their purpose. ** Note: A key design decision can be seen in the ImportJobTestConfiguration interface ** - I decided to create a new function to get the expected results for each file format, since we seldom add new fileformats. - However this also enforces future configurations to always define their expected result for every file forma or throw a NotImplementedException should they lack the support for one. - The alternative for this is to define the fileLayout as an input parameter instead. This would allow for better extendability. _Please share your thoughts on this!_ Diffs (updated) ----- src/java/org/apache/sqoop/config/ConfigurationConstants.java 3724f250e src/java/org/apache/sqoop/mapreduce/ImportJobBase.java 80c069888 src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java 62334f8ab src/java/org/apache/sqoop/orm/AvroSchemaGenerator.java 7a2a5f9cd src/test/org/apache/sqoop/importjob/ImportJobTestConfiguration.java 14de910b9 src/test/org/apache/sqoop/importjob/SplitByImportTest.java 7977c0b0f src/test/org/apache/sqoop/importjob/avro/AvroImportForNumericTypesTest.java ff13dc3bc src/test/org/apache/sqoop/importjob/avro/configuration/MSSQLServerImportJobTestConfiguration.java 182d2967f src/test/org/apache/sqoop/importjob/avro/configuration/MySQLImportJobTestConfiguration.java e9bf9912a src/test/org/apache/sqoop/importjob/avro/configuration/OracleImportJobTestConfiguration.java b7bad08c0 src/test/org/apache/sqoop/importjob/avro/configuration/OracleImportJobTestConfigurationForNumber.java 465e61f4b src/test/org/apache/sqoop/importjob/avro/configuration/PostgresqlImportJobTestConfigurationForNumeric.java 66715c171 src/test/org/apache/sqoop/importjob/avro/configuration/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java ec4db41bd src/test/org/apache/sqoop/importjob/configuration/AvroTestConfiguration.java PRE-CREATION src/test/org/apache/sqoop/importjob/configuration/GenericImportJobSplitByTestConfiguration.java f137b56b7 src/test/org/apache/sqoop/importjob/configuration/ParquetTestConfiguration.java PRE-CREATION src/test/org/apache/sqoop/util/ParquetReader.java 908ce566f Diff: https://reviews.apache.org/r/69060/diff/3/ Changes: https://reviews.apache.org/r/69060/diff/2-3/ Testing ------- 3rd party tests and unit tests, both gradle and ant Thanks, Fero Szabo