You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Nick White <nw...@palantir.com> on 2013/07/28 20:05:44 UTC

Re: Review Request 12949: SQOOP-1149: Support custom postgres types (e.g. inet for IP addresses) - which includes postgres enums.

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

(Updated July 28, 2013, 6:05 p.m.)


Review request for Sqoop.


Changes
-------

I've revised the patch to maintain backwards compatibility. Maybe Sqoop 2.0 should hide how the types are gathered (e.g. from a table / query / stored procedure) from a database? The patch overrides toJavaType in PostgresqlManager as you suggest, but I still have to tweak the generated SQL when exporting as the postgres driver gives all custom types an SQL type of 1111 (OTHER). Postgres doesn't automatically convert Strings to custom types (e.g. http://stackoverflow.com/questions/3080470/jdbctemplate-and-inet-data-type); it needs explicit casts. Thanks -


Summary (updated)
-----------------

SQOOP-1149: Support custom postgres types (e.g. inet for IP addresses) - which includes postgres enums.


Repository: sqoop-trunk


Description
-------

The patch adds a PostgresqlExportJob that replaces the OutputFormat (if needed) with a PostgresqlExportOutputFormat that inserts casts into the generated SQL statement (e.g. insert into mytable values (?, ?::inet, ?)). The patch also consolidates the various functions on ConnManager that return SQL type ints and type names into just one for each. This means the chunks of code in various parts of the codebase that select which of the three (former) methods to call can be replaced with a single call - and the call "routing" logic only appears in one place (ConnManager).


Diffs (updated)
-----

  src/java/org/apache/sqoop/manager/ConnManager.java f4b22f9 
  src/java/org/apache/sqoop/manager/PostgresqlManager.java bd882b9 
  src/java/org/apache/sqoop/mapreduce/ExportOutputFormat.java c2e39b1 
  src/java/org/apache/sqoop/mapreduce/PostgresqlExportJob.java PRE-CREATION 
  src/java/org/apache/sqoop/mapreduce/PostgresqlExportOutputFormat.java PRE-CREATION 
  src/test/com/cloudera/sqoop/manager/PostgresqlExportTest.java 0ac4599 

Diff: https://reviews.apache.org/r/12949/diff/


Testing
-------

I've added two cases to PostgresqlExportTest and tested them against a 9.3 database.


Thanks,

Nick White


Re: Review Request 12949: SQOOP-1149: Support custom postgres types (e.g. inet for IP addresses) - which includes postgres enums.

Posted by Nick White <nw...@palantir.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12949/
-----------------------------------------------------------

(Updated Aug. 1, 2013, 3:47 p.m.)


Review request for Sqoop.


Changes
-------

(removed unneeded cast)


Repository: sqoop-trunk


Description
-------

The patch adds a PostgresqlExportJob that replaces the OutputFormat (if needed) with a PostgresqlExportOutputFormat that inserts casts into the generated SQL statement (e.g. insert into mytable values (?, ?::inet, ?)). The patch also consolidates the various functions on ConnManager that return SQL type ints and type names into just one for each. This means the chunks of code in various parts of the codebase that select which of the three (former) methods to call can be replaced with a single call - and the call "routing" logic only appears in one place (ConnManager).


Diffs (updated)
-----

  src/java/com/cloudera/sqoop/lib/JdbcWritableBridge.java 316547f 
  src/java/org/apache/sqoop/lib/JdbcWritableBridge.java afde585 
  src/java/org/apache/sqoop/manager/ConnManager.java f4b22f9 
  src/java/org/apache/sqoop/manager/PostgresqlManager.java bd882b9 
  src/java/org/apache/sqoop/orm/ClassWriter.java 1bd2a41 
  src/test/com/cloudera/sqoop/manager/PostgresqlExportTest.java 0ac4599 

Diff: https://reviews.apache.org/r/12949/diff/


Testing
-------

I've added two cases to PostgresqlExportTest and tested them against a 9.3 database.


Thanks,

Nick White


Re: Review Request 12949: SQOOP-1149: Support custom postgres types (e.g. inet for IP addresses) - which includes postgres enums.

Posted by Nick White <nw...@palantir.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12949/
-----------------------------------------------------------

(Updated Aug. 1, 2013, 3:43 p.m.)


Review request for Sqoop.


Changes
-------

I think I've come up with a nice way to abstract driver-specific Object types from the ClassWriter - see the attached (backwards-compatible) patch.


Repository: sqoop-trunk


Description
-------

The patch adds a PostgresqlExportJob that replaces the OutputFormat (if needed) with a PostgresqlExportOutputFormat that inserts casts into the generated SQL statement (e.g. insert into mytable values (?, ?::inet, ?)). The patch also consolidates the various functions on ConnManager that return SQL type ints and type names into just one for each. This means the chunks of code in various parts of the codebase that select which of the three (former) methods to call can be replaced with a single call - and the call "routing" logic only appears in one place (ConnManager).


Diffs (updated)
-----

  src/java/com/cloudera/sqoop/lib/JdbcWritableBridge.java 316547f 
  src/java/org/apache/sqoop/lib/JdbcWritableBridge.java afde585 
  src/java/org/apache/sqoop/manager/ConnManager.java f4b22f9 
  src/java/org/apache/sqoop/manager/PostgresqlManager.java bd882b9 
  src/java/org/apache/sqoop/orm/ClassWriter.java 1bd2a41 
  src/test/com/cloudera/sqoop/manager/PostgresqlExportTest.java 0ac4599 

Diff: https://reviews.apache.org/r/12949/diff/


Testing
-------

I've added two cases to PostgresqlExportTest and tested them against a 9.3 database.


Thanks,

Nick White


Re: Review Request 12949: SQOOP-1149: Support custom postgres types (e.g. inet for IP addresses) - which includes postgres enums.

Posted by Nick White <nw...@palantir.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12949/
-----------------------------------------------------------

(Updated July 29, 2013, 1:37 a.m.)


Review request for Sqoop.


Changes
-------

Actually, I've been thinking about this a bit more, and have come up with the attached. Instead of re-writing the SQL INSERT statement to insert casts, it just uses the postgres driver's generic object class, org.postgresql.util.PGobject, in the generated code. The JdbcWritableBridge passes these PGobjects to and from a ResultSet's readObject / writeObject methods without needing to modifying them. The only downside to this approach is that ClassWriter uses the implementation-specific PGobject class. Can you think of a way around this? Maybe the ConnManager subclasses should know how to generate code for their database's object types?


Repository: sqoop-trunk


Description
-------

The patch adds a PostgresqlExportJob that replaces the OutputFormat (if needed) with a PostgresqlExportOutputFormat that inserts casts into the generated SQL statement (e.g. insert into mytable values (?, ?::inet, ?)). The patch also consolidates the various functions on ConnManager that return SQL type ints and type names into just one for each. This means the chunks of code in various parts of the codebase that select which of the three (former) methods to call can be replaced with a single call - and the call "routing" logic only appears in one place (ConnManager).


Diffs (updated)
-----

  src/java/com/cloudera/sqoop/lib/JdbcWritableBridge.java 316547f 
  src/java/org/apache/sqoop/lib/JdbcWritableBridge.java afde585 
  src/java/org/apache/sqoop/manager/PostgresqlManager.java bd882b9 
  src/java/org/apache/sqoop/orm/ClassWriter.java 1bd2a41 
  src/test/com/cloudera/sqoop/manager/PostgresqlExportTest.java 0ac4599 

Diff: https://reviews.apache.org/r/12949/diff/


Testing
-------

I've added two cases to PostgresqlExportTest and tested them against a 9.3 database.


Thanks,

Nick White