Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-18 Thread Szabolcs Vasas 

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


Ship it!




Ship It!

- Szabolcs Vasas


On April 17, 2018, 2:42 p.m., Fero Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66446/
> ---
> 
> (Updated April 17, 2018, 2:42 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-2567
> https://issues.apache.org/jira/browse/SQOOP-2567
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This fix allows the user to specify default precision and scale for avro 
> schemas. The default values are then used to override "invalid" values, (when 
> the database returns 0s as precision) and in case of oracle, the -127 scale 
> value. 
> 
> **Key points**
> - The implementation takes place in the ConnManager#toAvroLogicalType 
> function and the overriding funcitons in OraOopConnManager and OracleManager
> - Testing is covered very thoroughly by the TestAvroImportForNumericTypes 
> class and multiple configurations are used to cover MySQL, Oracle, Postgres 
> and MS SQL.
> 
> **Implementation specific concerns**
> - The edge cases aren't well documented. These tests aim to cover the 
> NUMBER/NUMERIC and DECIMAL types with or without specified scale and 
> precision thoroughly. Are there any missed testcases?
> - The new parameters act as overrides only for PSQL and Oracle databases, 
> because we the other databases translate the missing precision to valid 
> values. Even though this is true, I've added testcases for MS SQL and MySQL.
> 
> - In case of Oracle 
> The databae returns if user doesn't specify the default scale and the db 
> return -127, we adjust the precision by that much.
> Should we throw an exception instead?
> 
> - The default precision has to be specified. If it's not there and the 
> database returns 0 we throw an exception. 
> - Instead, if the default precision and scale aren't there, we could just use 
> the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
> that would fit everything in a very inefficient manner, mostly containing 0s. 
> (This also opens up the question whether there is an efficient way to store 
> numbers with many 0s in avro.)
> 
> **Testing specific concerns**
> - The ImportJobTestConfiguration#dropTableIfExists method is not really a 
> test configuration related method, however at the time of development, it 
> made sense to have it there. This might be better off in another place, such 
> as BaseSqoopTest (though I'm unsure how that implementation would look like.)
> - The SqlUtil class was created solely to provide a place for the 
> executeStatement method. This might also be better off in another class, such 
> as BaseSqoopTest.
> 
> 
> Diffs
> -
> 
>   src/docs/user/import.txt e91a5a84 
>   src/java/org/apache/sqoop/avro/AvroUtil.java caed90ea 
>   src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
>   src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
>   src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
>   src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
>   src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
>   src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
>   src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
>   src/test/org/apache/sqoop/importjob/ImportJobTestConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/importjob/avro/AvroImportForNumericTypesTest.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/importjob/avro/configuration/MSSQLServerImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/importjob/avro/configuration/MySQLImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/importjob/avro/configuration/OracleImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/importjob/avro/configuration/OracleImportJobTestConfigurationForNumber.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/importjob/avro/configuration/PostgresqlImportJobTestConfigurationForNumeric.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/importjob/avro/configuration/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java 
> a6121c9a 
>   src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 75ecc357 
>   src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java 
> f217f0bc 
>   src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 6d752aa4 
>

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-17 Thread Fero Szabo via Review Board 

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

(Updated April 17, 2018, 1:15 p.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Bugs: SQOOP-2567
https://issues.apache.org/jira/browse/SQOOP-2567


Repository: sqoop-trunk


Description
---

This fix allows the user to specify default precision and scale for avro 
schemas. The default values are then used to override "invalid" values, (when 
the database returns 0s as precision) and in case of oracle, the -127 scale 
value. 

**Key points**
- The implementation takes place in the ConnManager#toAvroLogicalType function 
and the overriding funcitons in OraOopConnManager and OracleManager
- Testing is covered very thoroughly by the TestAvroImportForNumericTypes class 
and multiple configurations are used to cover MySQL, Oracle, Postgres and MS 
SQL.

**Implementation specific concerns**
- The edge cases aren't well documented. These tests aim to cover the 
NUMBER/NUMERIC and DECIMAL types with or without specified scale and precision 
thoroughly. Are there any missed testcases?
- The new parameters act as overrides only for PSQL and Oracle databases, 
because we the other databases translate the missing precision to valid values. 
Even though this is true, I've added testcases for MS SQL and MySQL.

- In case of Oracle 
The databae returns if user doesn't specify the default scale and the db return 
-127, we adjust the precision by that much.
Should we throw an exception instead?

- The default precision has to be specified. If it's not there and the database 
returns 0 we throw an exception. 
- Instead, if the default precision and scale aren't there, we could just use 
the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
that would fit everything in a very inefficient manner, mostly containing 0s. 
(This also opens up the question whether there is an efficient way to store 
numbers with many 0s in avro.)

**Testing specific concerns**
- The ImportJobTestConfiguration#dropTableIfExists method is not really a test 
configuration related method, however at the time of development, it made sense 
to have it there. This might be better off in another place, such as 
BaseSqoopTest (though I'm unsure how that implementation would look like.)
- The SqlUtil class was created solely to provide a place for the 
executeStatement method. This might also be better off in another class, such 
as BaseSqoopTest.


Diffs (updated)
-

  src/docs/user/import.txt e91a5a84 
  src/java/org/apache/sqoop/avro/AvroUtil.java caed90ea 
  src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
  src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
  src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
  src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
  src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
  src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
  src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
  src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MSSQLServerImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MySQLImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfigurationForNumber.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationForNumeric.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java
 PRE-CREATION 
  src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java a6121c9a 
  src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 75ecc357 
  src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java 
f217f0bc 
  src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 6d752aa4 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlImportTest.java 
846228a1 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlTestUtil.java 
PRE-CREATION 
  src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 2220b7d5 
  
src/test/org/apache/sqoop/manager/sqlserver/SQLServerAvroPaddingImportTest.java 
27dc0cd7 
  src/test/org/apache/sqoop/testutil/SqlUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/testutil/adapter/DatabaseAdapter.java PRE-CREATION 
  src/test/org/apache/sqoop/testutil/adapter/MSSQLServerDatabaseAdapter.java 
PRE-CREATION

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-17 Thread Fero Szabo via Review Board 

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

(Updated April 17, 2018, 1:11 p.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Bugs: SQOOP-2567
https://issues.apache.org/jira/browse/SQOOP-2567


Repository: sqoop-trunk


Description
---

This fix allows the user to specify default precision and scale for avro 
schemas. The default values are then used to override "invalid" values, (when 
the database returns 0s as precision) and in case of oracle, the -127 scale 
value. 

**Key points**
- The implementation takes place in the ConnManager#toAvroLogicalType function 
and the overriding funcitons in OraOopConnManager and OracleManager
- Testing is covered very thoroughly by the TestAvroImportForNumericTypes class 
and multiple configurations are used to cover MySQL, Oracle, Postgres and MS 
SQL.

**Implementation specific concerns**
- The edge cases aren't well documented. These tests aim to cover the 
NUMBER/NUMERIC and DECIMAL types with or without specified scale and precision 
thoroughly. Are there any missed testcases?
- The new parameters act as overrides only for PSQL and Oracle databases, 
because we the other databases translate the missing precision to valid values. 
Even though this is true, I've added testcases for MS SQL and MySQL.

- In case of Oracle 
The databae returns if user doesn't specify the default scale and the db return 
-127, we adjust the precision by that much.
Should we throw an exception instead?

- The default precision has to be specified. If it's not there and the database 
returns 0 we throw an exception. 
- Instead, if the default precision and scale aren't there, we could just use 
the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
that would fit everything in a very inefficient manner, mostly containing 0s. 
(This also opens up the question whether there is an efficient way to store 
numbers with many 0s in avro.)

**Testing specific concerns**
- The ImportJobTestConfiguration#dropTableIfExists method is not really a test 
configuration related method, however at the time of development, it made sense 
to have it there. This might be better off in another place, such as 
BaseSqoopTest (though I'm unsure how that implementation would look like.)
- The SqlUtil class was created solely to provide a place for the 
executeStatement method. This might also be better off in another class, such 
as BaseSqoopTest.


Diffs (updated)
-

  IMPORT_TABLE_1.java PRE-CREATION 
  IMPORT_TABLE_2.java PRE-CREATION 
  IMPORT_TABLE_3.java PRE-CREATION 
  IMPORT_TABLE_4.java PRE-CREATION 
  IMPORT_TABLE_5.java PRE-CREATION 
  IMPORT_TABLE_6.java PRE-CREATION 
  IMPORT_TABLE_7.java PRE-CREATION 
  IMPORT_TABLE_8.java PRE-CREATION 
  src/docs/user/import.txt e91a5a84 
  src/java/org/apache/sqoop/avro/AvroUtil.java caed90ea 
  src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
  src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
  src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
  src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
  src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
  src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
  src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
  src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MSSQLServerImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MySQLImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfigurationForNumber.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationForNumeric.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java
 PRE-CREATION 
  src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java a6121c9a 
  src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 75ecc357 
  src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java 
f217f0bc 
  src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 6d752aa4 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlImportTest.java 
846228a1 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlTestUtil.java 
PRE-CREATION 
  src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 2220b7d5

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-17 Thread Fero Szabo via Review Board 


> On April 16, 2018, 2:28 p.m., Szabolcs Vasas wrote:
> > src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java
> > Lines 131 (patched)
> > 
> >
> > Now that we have MySqlDatabaseAdapter shouldn't we move this method to 
> > that class?

As we discussed, I'd like to keep the scope of this change smaller, so we can 
do this refactor in another one.


- Fero


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


On April 17, 2018, 12:29 p.m., Fero Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66446/
> ---
> 
> (Updated April 17, 2018, 12:29 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-2567
> https://issues.apache.org/jira/browse/SQOOP-2567
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This fix allows the user to specify default precision and scale for avro 
> schemas. The default values are then used to override "invalid" values, (when 
> the database returns 0s as precision) and in case of oracle, the -127 scale 
> value. 
> 
> **Key points**
> - The implementation takes place in the ConnManager#toAvroLogicalType 
> function and the overriding funcitons in OraOopConnManager and OracleManager
> - Testing is covered very thoroughly by the TestAvroImportForNumericTypes 
> class and multiple configurations are used to cover MySQL, Oracle, Postgres 
> and MS SQL.
> 
> **Implementation specific concerns**
> - The edge cases aren't well documented. These tests aim to cover the 
> NUMBER/NUMERIC and DECIMAL types with or without specified scale and 
> precision thoroughly. Are there any missed testcases?
> - The new parameters act as overrides only for PSQL and Oracle databases, 
> because we the other databases translate the missing precision to valid 
> values. Even though this is true, I've added testcases for MS SQL and MySQL.
> 
> - In case of Oracle 
> The databae returns if user doesn't specify the default scale and the db 
> return -127, we adjust the precision by that much.
> Should we throw an exception instead?
> 
> - The default precision has to be specified. If it's not there and the 
> database returns 0 we throw an exception. 
> - Instead, if the default precision and scale aren't there, we could just use 
> the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
> that would fit everything in a very inefficient manner, mostly containing 0s. 
> (This also opens up the question whether there is an efficient way to store 
> numbers with many 0s in avro.)
> 
> **Testing specific concerns**
> - The ImportJobTestConfiguration#dropTableIfExists method is not really a 
> test configuration related method, however at the time of development, it 
> made sense to have it there. This might be better off in another place, such 
> as BaseSqoopTest (though I'm unsure how that implementation would look like.)
> - The SqlUtil class was created solely to provide a place for the 
> executeStatement method. This might also be better off in another class, such 
> as BaseSqoopTest.
> 
> 
> Diffs
> -
> 
>   src/docs/user/import.txt e91a5a84 
>   src/java/org/apache/sqoop/avro/AvroUtil.java caed90ea 
>   src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
>   src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
>   src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
>   src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
>   src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
>   src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
>   src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
>   src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java PRE-CREATION 
>   
> src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/configuration/importjob/avro/MSSQLServerImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/configuration/importjob/avro/MySQLImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfigurationForNumber.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationForNumeric.java
>  PRE-CREATION 
>   
>

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-17 Thread Fero Szabo via Review Board 

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

(Updated April 17, 2018, 12:29 p.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Bugs: SQOOP-2567
https://issues.apache.org/jira/browse/SQOOP-2567


Repository: sqoop-trunk


Description
---

This fix allows the user to specify default precision and scale for avro 
schemas. The default values are then used to override "invalid" values, (when 
the database returns 0s as precision) and in case of oracle, the -127 scale 
value. 

**Key points**
- The implementation takes place in the ConnManager#toAvroLogicalType function 
and the overriding funcitons in OraOopConnManager and OracleManager
- Testing is covered very thoroughly by the TestAvroImportForNumericTypes class 
and multiple configurations are used to cover MySQL, Oracle, Postgres and MS 
SQL.

**Implementation specific concerns**
- The edge cases aren't well documented. These tests aim to cover the 
NUMBER/NUMERIC and DECIMAL types with or without specified scale and precision 
thoroughly. Are there any missed testcases?
- The new parameters act as overrides only for PSQL and Oracle databases, 
because we the other databases translate the missing precision to valid values. 
Even though this is true, I've added testcases for MS SQL and MySQL.

- In case of Oracle 
The databae returns if user doesn't specify the default scale and the db return 
-127, we adjust the precision by that much.
Should we throw an exception instead?

- The default precision has to be specified. If it's not there and the database 
returns 0 we throw an exception. 
- Instead, if the default precision and scale aren't there, we could just use 
the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
that would fit everything in a very inefficient manner, mostly containing 0s. 
(This also opens up the question whether there is an efficient way to store 
numbers with many 0s in avro.)

**Testing specific concerns**
- The ImportJobTestConfiguration#dropTableIfExists method is not really a test 
configuration related method, however at the time of development, it made sense 
to have it there. This might be better off in another place, such as 
BaseSqoopTest (though I'm unsure how that implementation would look like.)
- The SqlUtil class was created solely to provide a place for the 
executeStatement method. This might also be better off in another class, such 
as BaseSqoopTest.


Diffs (updated)
-

  src/docs/user/import.txt e91a5a84 
  src/java/org/apache/sqoop/avro/AvroUtil.java caed90ea 
  src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
  src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
  src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
  src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
  src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
  src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
  src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
  src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MSSQLServerImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MySQLImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfigurationForNumber.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationForNumeric.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java
 PRE-CREATION 
  src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java a6121c9a 
  src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 75ecc357 
  src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java 
f217f0bc 
  src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 6d752aa4 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlImportTest.java 
846228a1 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlTestUtil.java 
PRE-CREATION 
  src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 2220b7d5 
  
src/test/org/apache/sqoop/manager/sqlserver/SQLServerAvroPaddingImportTest.java 
27dc0cd7 
  src/test/org/apache/sqoop/testutil/AvroTestUtils.java 75940bf1 
  src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java a5f85a06 
  src/test/org/apache/sqoop/testutil/SqlUtil.java PRE-CREATION

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-16 Thread Boglarka Egyed 

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



Hi Fero,

Thank you for this nice addition!

I have started to look into your change, you can find some minor findings 
below, but I ran into some failing unit tests in TestHsqldbAvroPadding, e.g.:

Testcase: testAvroImportWithPadding took 0.853 sec
FAILED
Could not insert into table: java.sql.SQLException: Unexpected token: AARON in 
statement [INSERT INTO "IMPORT_TABLE_1"("ID", "NAME", "SALARY", "DEPT") 
VALUES('1', ''Aaron'', '100.05', ''engineering'')]
at org.hsqldb.jdbc.Util.throwError(Unknown Source)
at org.hsqldb.jdbc.jdbcPreparedStatement.(Unknown Source)
at org.hsqldb.jdbc.jdbcConnection.prepareStatement(Unknown Source)
at 
org.apache.sqoop.testutil.BaseSqoopTestCase.insertIntoTable(BaseSqoopTestCase.java:466)
at 
org.apache.sqoop.manager.hsqldb.TestHsqldbAvroPadding.createTestTable(TestHsqldbAvroPadding.java:54)
at 
org.apache.sqoop.manager.hsqldb.TestHsqldbAvroPadding.setUp(TestHsqldbAvroPadding.java:46)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at 
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at 
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at 
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
at 
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at 
org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:239)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at 
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at junit.framework.JUnit4TestAdapter.run(JUnit4TestAdapter.java:38)
at 
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:535)
at 
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:1182)
at 
org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:1033)

junit.framework.AssertionFailedError: Could not insert into table: 
java.sql.SQLException: Unexpected token: AARON in statement [INSERT INTO 
"IMPORT_TABLE_1"("ID", "NAME", "SALARY", "DEPT") VALUES('1', ''Aaron'', 
'100.05', ''engineering'')]
at org.hsqldb.jdbc.Util.throwError(Unknown Source)
at org.hsqldb.jdbc.jdbcPreparedStatement.(Unknown Source)
at org.hsqldb.jdbc.jdbcConnection.prepareStatement(Unknown Source)
at 
org.apache.sqoop.testutil.BaseSqoopTestCase.insertIntoTable(BaseSqoopTestCase.java:466)
at 
org.apache.sqoop.manager.hsqldb.TestHsqldbAvroPadding.createTestTable(TestHsqldbAvroPadding.java:54)
at 
org.apache.sqoop.manager.hsqldb.TestHsqldbAvroPadding.setUp(TestHsqldbAvroPadding.java:46)

at 
org.apache.sqoop.testutil.BaseSqoopTestCase.insertIntoTable(BaseSqoopTestCase.java:471)
at 
org.apache.sqoop.manager.hsqldb.TestHsqldbAvroPadding.createTestTable(TestHsqldbAvroPadding.java:54)
at 
org.apache.sqoop.manager.hsqldb.TestHsqldbAvroPadding.setUp(TestHsqldbAvroPadding.java:46)

Could you please take a look at this?

Thanks,
Bogi


src/java/org/apache/sqoop/manager/ConnManager.java
Line 230 (original), 234 (patched)


Parameters precision and scale are missing from this javadoc.



src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java
Lines 66-69 (patched)


Why aren't these fields final?



src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java
Lines 68 (patched)


typo: SUCCED


- Boglarka Egyed


On April 13, 2018, 3:45 p.m., Fero

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-16 Thread Szabolcs Vasas 

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



Hi Feró,

Thank you for fixing my findings, I think the code generally looks now I have 
left some minor comments.


src/java/org/apache/sqoop/manager/oracle/OracleUtils.java
Lines 86 (patched)


This method looks really similar to 
org.apache.sqoop.manager.ConnManager#toAvroLogicalType(int, java.lang.Integer, 
java.lang.Integer). Can we somehow reorganize the code to contain less 
duplications?



src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java
Lines 54 (patched)


I think it would be good to add some comments to this test case to clarify 
what exactly it wants to test.



src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java
Lines 68 (patched)


typo: SUCCEED



src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java
Lines 71 (patched)


It is a small thing but I think it would improve the readability of the 
test results if you would override the toString() method of the adapters and 
the configuration classes to return the simple name of the class, currently the 
parameter name gets really long.



src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java
Lines 89 (patched)


The variable naming is confusing here.



src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
Lines 23 (patched)


Is there a reason this abstract class is not an interface?

Is this class hierarchy meant to be used in other test cases too or only in 
AvroImportForNumericTypesTest? If latter then I think it should be moved to the 
same package as AvroImportForNumericTypesTest and the name should be less 
generic.



src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java
Lines 131 (patched)


Now that we have MySqlDatabaseAdapter shouldn't we move this method to that 
class?



src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java
Lines 24 (patched)


Unused import.



src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java
Lines 26 (patched)


Unused import.



src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java
Lines 27 (patched)


Unused import.



src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java
Lines 31 (patched)


Unused import.



src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java
Lines 33 (patched)


Unused import.



src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java
Lines 34 (patched)


Unused import.



src/test/org/apache/sqoop/testutil/adapter/DatabaseAdapter.java
Lines 28 (patched)


nit: there are 2 spaces before the method name.



src/test/org/apache/sqoop/testutil/adapter/DatabaseAdapter.java
Lines 30 (patched)


nit: public is redundant for interface methods.



src/test/org/apache/sqoop/testutil/adapter/MSSQLServerDatabaseAdapter.java
Lines 27 (patched)


Formatting: 2 spaces before DatabaseAdapter but no space before {



src/test/org/apache/sqoop/testutil/adapter/OracleDatabaseAdapter.java
Lines 28 (patched)


Formatting: 2 spaces before DatabaseAdapter



src/test/org/apache/sqoop/tool/ImportToolValidateOptionsTest.java
Line 42 (original), 42 (patched)


This reformatting does not seem to be related to this patch, can you please 
remove it?


- Szabolcs Vasas


On April 13, 2018, 3:45 p.m., Fero Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66446/
> ---
> 
> (Updated April 13, 2018, 3:45 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-2567
> https://issues.apache.org/jira/browse/SQOOP-2567
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This fix allows the user to specify default precision and scale for avro 
> schemas. The

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-13 Thread Fero Szabo via Review Board 

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

(Updated April 13, 2018, 3:45 p.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Changes
---

I also added documentation for this new feature


Bugs: SQOOP-2567
https://issues.apache.org/jira/browse/SQOOP-2567


Repository: sqoop-trunk


Description
---

This fix allows the user to specify default precision and scale for avro 
schemas. The default values are then used to override "invalid" values, (when 
the database returns 0s as precision) and in case of oracle, the -127 scale 
value. 

**Key points**
- The implementation takes place in the ConnManager#toAvroLogicalType function 
and the overriding funcitons in OraOopConnManager and OracleManager
- Testing is covered very thoroughly by the TestAvroImportForNumericTypes class 
and multiple configurations are used to cover MySQL, Oracle, Postgres and MS 
SQL.

**Implementation specific concerns**
- The edge cases aren't well documented. These tests aim to cover the 
NUMBER/NUMERIC and DECIMAL types with or without specified scale and precision 
thoroughly. Are there any missed testcases?
- The new parameters act as overrides only for PSQL and Oracle databases, 
because we the other databases translate the missing precision to valid values. 
Even though this is true, I've added testcases for MS SQL and MySQL.

- In case of Oracle 
The databae returns if user doesn't specify the default scale and the db return 
-127, we adjust the precision by that much.
Should we throw an exception instead?

- The default precision has to be specified. If it's not there and the database 
returns 0 we throw an exception. 
- Instead, if the default precision and scale aren't there, we could just use 
the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
that would fit everything in a very inefficient manner, mostly containing 0s. 
(This also opens up the question whether there is an efficient way to store 
numbers with many 0s in avro.)

**Testing specific concerns**
- The ImportJobTestConfiguration#dropTableIfExists method is not really a test 
configuration related method, however at the time of development, it made sense 
to have it there. This might be better off in another place, such as 
BaseSqoopTest (though I'm unsure how that implementation would look like.)
- The SqlUtil class was created solely to provide a place for the 
executeStatement method. This might also be better off in another class, such 
as BaseSqoopTest.


Diffs (updated)
-

  src/docs/user/import.txt e91a5a84 
  src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
  src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
  src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
  src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
  src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
  src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
  src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
  src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MSSQLServerImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MySQLImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfigurationForNumber.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationForNumeric.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java
 PRE-CREATION 
  src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java a6121c9a 
  src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 75ecc357 
  src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java 
f217f0bc 
  src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 6d752aa4 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlImportTest.java 
846228a1 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlTestUtil.java 
PRE-CREATION 
  src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 2220b7d5 
  
src/test/org/apache/sqoop/manager/sqlserver/SQLServerAvroPaddingImportTest.java 
27dc0cd7 
  src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java a5f85a06 
  src/test/org/apache/sqoop/testutil/SqlUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/testutil/adapter/DatabaseAdapter.java PRE-CREATION

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-13 Thread Fero Szabo via Review Board 

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

(Updated April 13, 2018, 3:02 p.m.)


Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.


Changes
---

Hi Szabi,

Thank you for your insights, I believe this is a much better design now!


Bugs: SQOOP-2567
https://issues.apache.org/jira/browse/SQOOP-2567


Repository: sqoop-trunk


Description
---

This fix allows the user to specify default precision and scale for avro 
schemas. The default values are then used to override "invalid" values, (when 
the database returns 0s as precision) and in case of oracle, the -127 scale 
value. 

**Key points**
- The implementation takes place in the ConnManager#toAvroLogicalType function 
and the overriding funcitons in OraOopConnManager and OracleManager
- Testing is covered very thoroughly by the TestAvroImportForNumericTypes class 
and multiple configurations are used to cover MySQL, Oracle, Postgres and MS 
SQL.

**Implementation specific concerns**
- The edge cases aren't well documented. These tests aim to cover the 
NUMBER/NUMERIC and DECIMAL types with or without specified scale and precision 
thoroughly. Are there any missed testcases?
- The new parameters act as overrides only for PSQL and Oracle databases, 
because we the other databases translate the missing precision to valid values. 
Even though this is true, I've added testcases for MS SQL and MySQL.

- In case of Oracle 
The databae returns if user doesn't specify the default scale and the db return 
-127, we adjust the precision by that much.
Should we throw an exception instead?

- The default precision has to be specified. If it's not there and the database 
returns 0 we throw an exception. 
- Instead, if the default precision and scale aren't there, we could just use 
the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
that would fit everything in a very inefficient manner, mostly containing 0s. 
(This also opens up the question whether there is an efficient way to store 
numbers with many 0s in avro.)

**Testing specific concerns**
- The ImportJobTestConfiguration#dropTableIfExists method is not really a test 
configuration related method, however at the time of development, it made sense 
to have it there. This might be better off in another place, such as 
BaseSqoopTest (though I'm unsure how that implementation would look like.)
- The SqlUtil class was created solely to provide a place for the 
executeStatement method. This might also be better off in another class, such 
as BaseSqoopTest.


Diffs (updated)
-

  src/java/org/apache/sqoop/config/ConfigurationConstants.java 2197025b 
  src/java/org/apache/sqoop/config/ConfigurationHelper.java e07a6998 
  src/java/org/apache/sqoop/manager/ConnManager.java d88b59bd 
  src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
  src/java/org/apache/sqoop/manager/SqlManager.java fe997c5f 
  src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 09207bb4 
  src/java/org/apache/sqoop/manager/oracle/OracleUtils.java aa56e708 
  src/test/org/apache/sqoop/AvroImportForNumericTypesTest.java PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/ImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MSSQLServerImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/MySQLImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfiguration.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/OracleImportJobTestConfigurationForNumber.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationForNumeric.java
 PRE-CREATION 
  
src/test/org/apache/sqoop/configuration/importjob/avro/PostgresqlImportJobTestConfigurationPaddingShouldSucceed.java
 PRE-CREATION 
  src/test/org/apache/sqoop/manager/mysql/MySQLLobAvroImportTest.java a6121c9a 
  src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 75ecc357 
  src/test/org/apache/sqoop/manager/oracle/OracleAvroPaddingImportTest.java 
f217f0bc 
  src/test/org/apache/sqoop/manager/oracle/util/OracleUtils.java 6d752aa4 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlImportTest.java 
846228a1 
  src/test/org/apache/sqoop/manager/postgresql/PostgresqlTestUtil.java 
PRE-CREATION 
  src/test/org/apache/sqoop/manager/sqlserver/MSSQLTestUtils.java 2220b7d5 
  
src/test/org/apache/sqoop/manager/sqlserver/SQLServerAvroPaddingImportTest.java 
27dc0cd7 
  src/test/org/apache/sqoop/testutil/BaseSqoopTestCase.java a5f85a06 
  src/test/org/apache/sqoop/testutil/SqlUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/testutil/adapter/DatabaseAdapter.java PRE-CREATION

Re: Review Request 66446: SQOOP-2567 SQOOP import for Oracle fails with invalid precision/scale for decimal

2018-04-12 Thread Szabolcs Vasas 

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



Hi Feró,

Thank you for submitting this patch, please find my findings inline.


src/java/org/apache/sqoop/config/ConfigurationHelper.java
Lines 255 (patched)


This variable can have a less specific name.



src/java/org/apache/sqoop/manager/oracle/OracleUtils.java
Lines 21 (patched)


Please remove unused imports.



src/test/org/apache/sqoop/TestAvroImportForNumericTypes.java
Lines 59 (patched)


Nice solution to parameterize the test cases instead of introducing 
inheritance!
I have a couple of suggestions here:
- ImportJobTestConfiguration contains methods which could be reused across 
different test cases (e.g. dropTableIfExists) and methods which are specific to 
TestAvroImportForNumericTypes. I think these should be split into seprate 
hierarchies and packages.
- I would also move the failWithoutPadding and failWithoutDefaults boolean 
values from the configuration classes and provide them as a separate parameter 
to the TestAvroImportForNumericTypes. This would improve the readability of the 
test since one would not have to navigate to another classes to determine if a 
test case should succeed or fail.
- If a test case succeeds with a specific 
org.apache.sqoop.testutil.configuration.ImportJobTestConfiguration#getTypes 
array that basically means that all of the enumerated data types work. However 
if a test case fails with such an array it means that  Sqoop does not work with 
at least one of these types but it is not visible which one. I think it would 
be great if we could somehow separate these data types into separate 
configurations maybe.



src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java
Lines 26 (patched)


Unused import.


- Szabolcs Vasas


On April 12, 2018, 10:06 a.m., Fero Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66446/
> ---
> 
> (Updated April 12, 2018, 10:06 a.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-2567
> https://issues.apache.org/jira/browse/SQOOP-2567
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This fix allows the user to specify default precision and scale for avro 
> schemas. The default values are then used to override "invalid" values, (when 
> the database returns 0s as precision) and in case of oracle, the -127 scale 
> value. 
> 
> **Key points**
> - The implementation takes place in the ConnManager#toAvroLogicalType 
> function and the overriding funcitons in OraOopConnManager and OracleManager
> - Testing is covered very thoroughly by the TestAvroImportForNumericTypes 
> class and multiple configurations are used to cover MySQL, Oracle, Postgres 
> and MS SQL.
> 
> **Implementation specific concerns**
> - The edge cases aren't well documented. These tests aim to cover the 
> NUMBER/NUMERIC and DECIMAL types with or without specified scale and 
> precision thoroughly. Are there any missed testcases?
> - The new parameters act as overrides only for PSQL and Oracle databases, 
> because we the other databases translate the missing precision to valid 
> values. Even though this is true, I've added testcases for MS SQL and MySQL.
> 
> - In case of Oracle 
> The databae returns if user doesn't specify the default scale and the db 
> return -127, we adjust the precision by that much.
> Should we throw an exception instead?
> 
> - The default precision has to be specified. If it's not there and the 
> database returns 0 we throw an exception. 
> - Instead, if the default precision and scale aren't there, we could just use 
> the maximum possible value i.e. 38 + 127 = 165 as precision and 127 as scale, 
> that would fit everything in a very inefficient manner, mostly containing 0s. 
> (This also opens up the question whether there is an efficient way to store 
> numbers with many 0s in avro.)
> 
> **Testing specific concerns**
> - The ImportJobTestConfiguration#dropTableIfExists method is not really a 
> test configuration related method, however at the time of development, it 
> made sense to have it there. This might be better off in another place, such 
> as BaseSqoopTest (though I'm unsure how that implementation would look like.)
> - The SqlUtil class was created solely to provide a place for the 
> executeStatement method. This might also be better off in another class, such 
> as BaseSqoopTest.
> 
> 
> Diffs
> -
> 
>