sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <b...@apache.org>
Subject Re: Review Request 65607: SQOOP-2976 Flag to expand decimal values to fit AVRO schema
Date Wed, 14 Feb 2018 10:54:58 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65607/#review197504
-----------------------------------------------------------


Ship it!




Hi Fero, 

Thank you for all the reivew finding corrections!

Tests still pass with your patch and it looks good to me know.

Please don't forget to open a separate JIRA for the documentation part.

Many thanks,
Bogi

- Boglarka Egyed


On Feb. 14, 2018, 9:45 a.m., Fero Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65607/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2018, 9:45 a.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-2976
>     https://issues.apache.org/jira/browse/SQOOP-2976
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> **Summary:**
> Certain databases, such as SQL Server and Postgres are storing decimal values padded
with 0s, should the user insert them with less digits than the given scale. 
> 
> Other databases however, such as Oracle and HSQLDB store these numbers without trailing
0s. Then, when the JDBC driver returns these as BigDecimals, they won't match the scale in
the avro schema.
> 
> Take the following SQL commands for an example: 
> ```
> create table salary (id int, amount number (10,5));
> insert into salary (id, amount) values (1, 10.5);
> insert into salary (id, amount) values (2, 10.50);
> select * from salary;
> ```
> Records in an Oracle database:
> 1	10.5
> 2	10.5
> 
> Records in SQL Server (using decimal instead of number in the create statement):
> 1	10.50000
> 2	10.50000
> 
> **Solition:**
> The fix is simply checking the scale of the returned BigDecimals against what's in the
avro schema and recreates the objects in case of a mismatch. I've introduced a new property
to enable this new feature, so existing behavior is not affected. 
> 
> **Notes: **
> - trimmings cannot happen, as we generate our schema based on the database columns. Therefore,
the values passed to AvroUtil#toAvro will have less or equal scale as the avro schema.
> - I will open a follow-up jira to document this change.
> 
> **Other notable changes:**
> - Introduced ArgumentArrayBuilder that reuses the existing Argument class and introduces
a useful builder pattern for creating commandline arguments for tests.
> - Slightly modified BaseSqoopTest to fit my needs. *(However, further refactoring would
be required in this class to enable better reuse. For example: the current implementation
can't be used with SQL Server, because one also needs to specify the schema besides the tablename
in the create and insert statements. There are also code duplications.)*
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/avro/AvroUtil.java 1aae8df2 
>   src/java/org/apache/sqoop/config/ConfigurationConstants.java 7a19a62c 
>   src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java a5e5bf5a 
>   src/test/org/apache/sqoop/TestAvroImport.java 1172fc5d 
>   src/test/org/apache/sqoop/manager/hsqldb/TestHsqldbAvroPadding.java PRE-CREATION 
>   src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java PRE-CREATION

>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerAvroPaddingImportTest.java PRE-CREATION

>   src/test/org/apache/sqoop/metastore/TestMetastoreConfigurationParameters.java 391dc336

>   src/test/org/apache/sqoop/testutil/ArgumentArrayBuilder.java PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/ArgumentUtils.java 2f95e455 
>   src/test/org/apache/sqoop/testutil/AvroTestUtils.java PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java 588f439c 
> 
> 
> Diff: https://reviews.apache.org/r/65607/diff/5/
> 
> 
> Testing
> -------
> 
> See the 3 new test classes (HSQLDB, Oracle, SQL Server).
> 
> 
> Thanks,
> 
> Fero Szabo
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message