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

Review request for Sqoop and Jarek Cecho.
Description
-------

commit b3b188e1b7a8fadf981ddaef4ffb8ec3676601dd
Author: Abraham Elmahrek <[EMAIL PROTECTED]>
Date:   Thu Mar 7 15:26:21 2013 -0800

    SQOOP-903 Add schema support to generic JDBC connector
    
    Schema is optional and job contexts are updated with proper table names.
    New tests for inclusion of schema and exclusion of schema.

:100644 100644 520b0bb... 6ee9a83... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java
:100644 100644 46c7ee7... 7bc0972... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
:100644 100644 718d1fb... ee4bb6e... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java
:100644 100644 d150779... 3422a8f... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
:100644 100644 6ab4296... 44fc984... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
:100644 100644 bb0c23b... a73f3e8... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
:100644 100644 45835bd... 778fe28... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
This addresses bug SQOOP-903.
    https://issues.apache.org/jira/browse/SQOOP-903
Diffs
-----

  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java 520b0bb
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 46c7ee7
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java 718d1fb
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java d150779
  connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 6ab4296
  connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java bb0c23b
  connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java 45835bd

Diff: https://reviews.apache.org/r/9808/diff/
Testing
-------

Created new test cases for testing Import and Export initialization in Generic JDBC Connector.
Tested import job with postgresql and schemas manually.
Thanks,

Abraham Elmahrek

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9808/#review17647
-----------------------------------------------------------
Hi Abe,
thank you very much for working on this. The patch looks good to me. I have just couple of comments:

1) Would you mind removing trailing white space characters? I've highlighted all of them for your convenience.

2) Would you mind adding Validation check in GenericJdbcValidator to ensure that the schema is not used with query based import?

3) I'm seeing failure in integration test TableImportTest, but that will be fixed by SQOOP-919, so it's not a show stopper.

Jarcec
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37426>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37427>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37428>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37429>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37430>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37431>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37432>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37433>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37434>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37435>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
<https://reviews.apache.org/r/9808/#comment37436>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37437>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37438>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37439>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37440>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37441>

    Nit: Trailing whitespace characters

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
<https://reviews.apache.org/r/9808/#comment37442>

    Nit: Trailing whitespace characters
- Jarek Cecho
On March 7, 2013, 11:46 p.m., Abraham Elmahrek wrote:

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

(Updated March 11, 2013, 7:33 p.m.)
Review request for Sqoop and Jarek Cecho.
Description
-------

commit b3b188e1b7a8fadf981ddaef4ffb8ec3676601dd
Author: Abraham Elmahrek <[EMAIL PROTECTED]>
Date:   Thu Mar 7 15:26:21 2013 -0800

    SQOOP-903 Add schema support to generic JDBC connector
    
    Schema is optional and job contexts are updated with proper table names.
    New tests for inclusion of schema and exclusion of schema.

:100644 100644 520b0bb... 6ee9a83... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java
:100644 100644 46c7ee7... 7bc0972... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
:100644 100644 718d1fb... ee4bb6e... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java
:100644 100644 d150779... 3422a8f... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
:100644 100644 6ab4296... 44fc984... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
:100644 100644 bb0c23b... a73f3e8... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
:100644 100644 45835bd... 778fe28... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
This addresses bug SQOOP-903.
    https://issues.apache.org/jira/browse/SQOOP-903
Diffs (updated)
-----

  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java 520b0bbce84d7eec63c25fc1165010a57892749d
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 46c7ee7c5c5329ab03e8e010f496f405d6ad2b2e
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java e098fbc1d155aad896e6960171d47c33c80c3baa
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java 718d1fbcf86cd4b04cdc5a6d30ac7ab1c654089a
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java d150779f2e61d9da4dca6cd3ac4e572a5fe55e46
  connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 6ab4296718b31501869f2f4dba922eae7d08b7ab
  connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java bb0c23b968fc58c2393f2a8d95b594bfc3b62100
  connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java 45835bd6f53411f2b1d2fbfbf772ed452d32904b

Diff: https://reviews.apache.org/r/9808/diff/
Testing
-------

Created new test cases for testing Import and Export initialization in Generic JDBC Connector.
Tested import job with postgresql and schemas manually.
Thanks,

Abraham Elmahrek

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

(Updated March 12, 2013, 12:31 a.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------

Remove hanging whitespace.
Description
-------

commit b3b188e1b7a8fadf981ddaef4ffb8ec3676601dd
Author: Abraham Elmahrek <[EMAIL PROTECTED]>
Date:   Thu Mar 7 15:26:21 2013 -0800

    SQOOP-903 Add schema support to generic JDBC connector
    
    Schema is optional and job contexts are updated with proper table names.
    New tests for inclusion of schema and exclusion of schema.

:100644 100644 520b0bb... 6ee9a83... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java
:100644 100644 46c7ee7... 7bc0972... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
:100644 100644 718d1fb... ee4bb6e... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java
:100644 100644 d150779... 3422a8f... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
:100644 100644 6ab4296... 44fc984... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
:100644 100644 bb0c23b... a73f3e8... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
:100644 100644 45835bd... 778fe28... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
This addresses bug SQOOP-903.
    https://issues.apache.org/jira/browse/SQOOP-903
Diffs (updated)
-----

  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java 520b0bbce84d7eec63c25fc1165010a57892749d
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 46c7ee7c5c5329ab03e8e010f496f405d6ad2b2e
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java e098fbc1d155aad896e6960171d47c33c80c3baa
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java 718d1fbcf86cd4b04cdc5a6d30ac7ab1c654089a
  connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java d150779f2e61d9da4dca6cd3ac4e572a5fe55e46
  connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 6ab4296718b31501869f2f4dba922eae7d08b7ab
  connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java bb0c23b968fc58c2393f2a8d95b594bfc3b62100
  connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java 45835bd6f53411f2b1d2fbfbf772ed452d32904b

Diff: https://reviews.apache.org/r/9808/diff/
Testing
-------

Created new test cases for testing Import and Export initialization in Generic JDBC Connector.
Tested import job with postgresql and schemas manually.
Thanks,

Abraham Elmahrek

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

Ship it!
Hi Abe,
thank you very much for putting this together. I do have just small nit to the validation message. Feel free to directly upload updated patch to the JIRA, I'll go ahead and commit it.
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java
<https://reviews.apache.org/r/9808/#comment37960>

    Nit: Would you putting this to the "schema" property instead of "table"?

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java
<https://reviews.apache.org/r/9808/#comment37961>

    Nit: Would you putting this to the "schema" property instead of "table"?
Jarcec

- Jarek Cecho
On March 12, 2013, 12:31 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9808/
> -----------------------------------------------------------
>
> (Updated March 12, 2013, 12:31 a.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> commit b3b188e1b7a8fadf981ddaef4ffb8ec3676601dd
> Author: Abraham Elmahrek <[EMAIL PROTECTED]>
> Date:   Thu Mar 7 15:26:21 2013 -0800
>
>     SQOOP-903 Add schema support to generic JDBC connector
>    
>     Schema is optional and job contexts are updated with proper table names.
>     New tests for inclusion of schema and exclusion of schema.
>
> :100644 100644 520b0bb... 6ee9a83... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java
> :100644 100644 46c7ee7... 7bc0972... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 718d1fb... ee4bb6e... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java
> :100644 100644 d150779... 3422a8f... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
> :100644 100644 6ab4296... 44fc984... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
> :100644 100644 bb0c23b... a73f3e8... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
> :100644 100644 45835bd... 778fe28... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
>
>
> This addresses bug SQOOP-903.
>     https://issues.apache.org/jira/browse/SQOOP-903
>
>
> Diffs
> -----
>
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java 520b0bbce84d7eec63c25fc1165010a57892749d
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 46c7ee7c5c5329ab03e8e010f496f405d6ad2b2e
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java e098fbc1d155aad896e6960171d47c33c80c3baa
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java 718d1fbcf86cd4b04cdc5a6d30ac7ab1c654089a
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java d150779f2e61d9da4dca6cd3ac4e572a5fe55e46
>   connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 6ab4296718b31501869f2f4dba922eae7d08b7ab
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java bb0c23b968fc58c2393f2a8d95b594bfc3b62100
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java 45835bd6f53411f2b1d2fbfbf772ed452d32904b

> On March 15, 2013, 4:50 a.m., Jarek Cecho wrote:
> > Hi Abe,
> > thank you very much for putting this together. I do have just small nit to the validation message. Feel free to directly upload updated patch to the JIRA, I'll go ahead and commit it.

Actually on second though, please let be as it is. Having this warning on form level does make better sense. Please attach your patch to the JIRA so that I can commit it.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9808/#review17953
-----------------------------------------------------------
On March 12, 2013, 12:31 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9808/
> -----------------------------------------------------------
>
> (Updated March 12, 2013, 12:31 a.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> commit b3b188e1b7a8fadf981ddaef4ffb8ec3676601dd
> Author: Abraham Elmahrek <[EMAIL PROTECTED]>
> Date:   Thu Mar 7 15:26:21 2013 -0800
>
>     SQOOP-903 Add schema support to generic JDBC connector
>    
>     Schema is optional and job contexts are updated with proper table names.
>     New tests for inclusion of schema and exclusion of schema.
>
> :100644 100644 520b0bb... 6ee9a83... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java
> :100644 100644 46c7ee7... 7bc0972... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 718d1fb... ee4bb6e... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java
> :100644 100644 d150779... 3422a8f... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
> :100644 100644 6ab4296... 44fc984... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
> :100644 100644 bb0c23b... a73f3e8... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java
> :100644 100644 45835bd... 778fe28... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java
>
>
> This addresses bug SQOOP-903.
>     https://issues.apache.org/jira/browse/SQOOP-903
>
>
> Diffs
> -----
>
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExportInitializer.java 520b0bbce84d7eec63c25fc1165010a57892749d
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 46c7ee7c5c5329ab03e8e010f496f405d6ad2b2e
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java e098fbc1d155aad896e6960171d47c33c80c3baa
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ExportTableForm.java 718d1fbcf86cd4b04cdc5a6d30ac7ab1c654089a
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java d150779f2e61d9da4dca6cd3ac4e572a5fe55e46
>   connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 6ab4296718b31501869f2f4dba922eae7d08b7ab
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExportInitializer.java bb0c23b968fc58c2393f2a8d95b594bfc3b62100
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportInitializer.java 45835bd6f53411f2b1d2fbfbf772ed452d32904b
>
> Diff: https://reviews.apache.org/r/9808/diff/
>
>
> Testing
> -------
>
> Created new test cases for testing Import and Export initialization in Generic JDBC Connector.