You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Abraham Elmahrek <ab...@cloudera.com> on 2014/10/20 20:47:13 UTC

Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

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

Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs
-----

  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5a8e026 
  pom.xml f25a29f 
  repository/pom.xml e3345c4 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 67baaa5 

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


Testing
-------

Can start Sqoop2 server.


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 7:55 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java, line 26
> > <https://reviews.apache.org/r/26948/diff/3/?file=779842#file779842line26>
> >
> >     It seems that the same file is in the patch twice, once as PostgreSQLTestUtils.java and then as PostgresqlTestUtils.java. They seems to have the same content so, let's nuke one of them?

Seems like a git thing. Bleh.


- Abraham


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


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/#review63892
-----------------------------------------------------------



repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java
<https://reviews.apache.org/r/26948/#comment106205>

    It seems that the same file is in the patch twice, once as PostgreSQLTestUtils.java and then as PostgresqlTestUtils.java. They seems to have the same content so, let's nuke one of them?


- Jarek Cecho


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java
> 
> Jarek Cecho wrote:
>     Please scratch 3), I found the clean up routine :)
> 
> Jarek Cecho wrote:
>     I've give it a spin on my real cluster and I can confirm that it's working just fine, thank you Abe!

Functional tests are in https://issues.apache.org/jira/browse/SQOOP-1591. Documentation is dependent on https://issues.apache.org/jira/browse/SQOOP-1680. Can create follow up on this though. Tests are removing entire schema in PostgresqlTestCase#tearDown. Is there any other clean up that you think might be useful?


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)

In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, lines 23-26
> > <https://reviews.apache.org/r/26948/diff/3/?file=779839#file779839line23>
> >
> >     I see the same very long comment in file PostgresqlSchemaCreateQuery. Let's perhaps have it in one single file and reference it in the other? I'm afraid that having it in several places will be harder for maintenance.

https://issues.apache.org/jira/browse/SQOOP-1824


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java, lines 44-48
> > <https://reviews.apache.org/r/26948/diff/3/?file=779841#file779841line44>
> >
> >     I'm a bit worried about this approach as it will hide configuration issues - e.g. if the database is down or credentials are missconfigured, the test will still pass and the fact that we are not testing PostgreSQL repo will be silently ignored.
> >     
> >     Let's perhaps promote those tests into Integration tests similarly as we're doing in test module [1] and add a flag to enable them?
> >     
> >     Another idea would be to use JUnit categories [2] and it's maven integration to run them in a special profile/flag.
> >     
> >     I'm happy to take this one into subsequent JIRA as it's more general topic though.
> >     
> >     Links:
> >     1: https://github.com/apache/sqoop/blob/sqoop2/test/pom.xml#L154
> >     2: https://weblogs.java.net/blog/johnsmart/archive/2010/04/25/grouping-tests-using-junit-categories-0
> >     3: http://maven.apache.org/surefire/maven-surefire-plugin/examples/junit.html

Interesting ideas! I've created SQOOP-1591 to address testing of this repository.


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, line 2203
> > <https://reviews.apache.org/r/26948/diff/3/?file=779830#file779830line2203>
> >
> >     Future: Perhaps a suggestion for future, I think that this can be further improved to simply use PrepareStatement.setObject() to support any type :)

Cool.


- Abraham


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


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java
> 
> Jarek Cecho wrote:
>     Please scratch 3), I found the clean up routine :)
> 
> Jarek Cecho wrote:
>     I've give it a spin on my real cluster and I can confirm that it's working just fine, thank you Abe!
> 
> Abraham Elmahrek wrote:
>     Functional tests are in https://issues.apache.org/jira/browse/SQOOP-1591. Documentation is dependent on https://issues.apache.org/jira/browse/SQOOP-1680. Can create follow up on this though. Tests are removing entire schema in PostgresqlTestCase#tearDown. Is there any other clean up that you think might be useful?

Thanks for the response Abe!

Fair enough, I'm fine with having the functional tests committed soon after as this separate and new component. I'm assuming that the tests will be added soon after this get in, correct?

The documentation JIRA SQOOP-1680 seems to be covering more an APIs for someone who is going to create new repository rather then instructing end user how to configure the repository. I know that all of the required properties are stored in "sqoop.properties" file and one just need to update them, but I was thinking about having a repository section in our admin guide that will list existing repositories and example configuration snipnets how to enable them.


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?

I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, lines 23-26
> > <https://reviews.apache.org/r/26948/diff/3/?file=779839#file779839line23>
> >
> >     I see the same very long comment in file PostgresqlSchemaCreateQuery. Let's perhaps have it in one single file and reference it in the other? I'm afraid that having it in several places will be harder for maintenance.
> 
> Abraham Elmahrek wrote:
>     https://issues.apache.org/jira/browse/SQOOP-1824

I'm fine with having the schema inside java file, because it's internal to the repository implementation. End user (and admin) should never go directly to the database and do changes there. I'm just a bit worried that keeping the copy in two places will get easily out of sync.


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java, lines 44-48
> > <https://reviews.apache.org/r/26948/diff/3/?file=779841#file779841line44>
> >
> >     I'm a bit worried about this approach as it will hide configuration issues - e.g. if the database is down or credentials are missconfigured, the test will still pass and the fact that we are not testing PostgreSQL repo will be silently ignored.
> >     
> >     Let's perhaps promote those tests into Integration tests similarly as we're doing in test module [1] and add a flag to enable them?
> >     
> >     Another idea would be to use JUnit categories [2] and it's maven integration to run them in a special profile/flag.
> >     
> >     I'm happy to take this one into subsequent JIRA as it's more general topic though.
> >     
> >     Links:
> >     1: https://github.com/apache/sqoop/blob/sqoop2/test/pom.xml#L154
> >     2: https://weblogs.java.net/blog/johnsmart/archive/2010/04/25/grouping-tests-using-junit-categories-0
> >     3: http://maven.apache.org/surefire/maven-surefire-plugin/examples/junit.html
> 
> Abraham Elmahrek wrote:
>     Interesting ideas! I've created SQOOP-1591 to address testing of this repository.

I've copy&pasted my comment to the SQOOP-1591 as that JIRA seemed to be missing all the details I've pointed out here :)


- Jarek


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


On Dec. 4, 2014, 9:18 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2014, 9:18 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.
> 
> Abraham Elmahrek wrote:
>     Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.
> 
> Abraham Elmahrek wrote:
>     The PostgreSQL repository does work with upper case since it seems to force names to lower case. The difficulty is in metadata queries it seems. If we query the metadata, then it will differ. What do you think Jarcec? Should we make an exception for Metadata queries? Handle these issues on a case-by-case basis?
> 
> Jarek Cecho wrote:
>     If we want to continue reusing the shared code, then another solution would be to alter our queries to properly enclose the table names, e.g. instead of
>     
>     SELECT * FROM table
>     
>     We should use
>     
>     SELECT * FROM "table"
>     
>     The small issue is that the enclose characters are different for each database :)
>     
>     Those small caveheats and differences have been the reason why we've decided to have one repository implementation per target database rather then having generic JDBC layer like some of the other projects have.
> 
> Abraham Elmahrek wrote:
>     That sounds good. I believe the same structure exists for the "Provider" interface as well.
> 
> Abraham Elmahrek wrote:
>     Do you think we can do this in a separate Jira given PostgreSQL repository appears to be working?
> 
> Jarek Cecho wrote:
>     The "Provider" is a test only though :)
>     
>     I see your point with doing this post-commit, so I'm fine with resolving it in separate JIRA. Could you file one Abe?

https://issues.apache.org/jira/browse/SQOOP-1945


- Abraham


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


On Dec. 18, 2014, 9:49 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2014, 9:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 609a875 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca590d8 
>   pom.xml aa4231e 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9fa2f9d 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 77477ee 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java
> 
> Jarek Cecho wrote:
>     Please scratch 3), I found the clean up routine :)
> 
> Jarek Cecho wrote:
>     I've give it a spin on my real cluster and I can confirm that it's working just fine, thank you Abe!
> 
> Abraham Elmahrek wrote:
>     Functional tests are in https://issues.apache.org/jira/browse/SQOOP-1591. Documentation is dependent on https://issues.apache.org/jira/browse/SQOOP-1680. Can create follow up on this though. Tests are removing entire schema in PostgresqlTestCase#tearDown. Is there any other clean up that you think might be useful?
> 
> Jarek Cecho wrote:
>     Thanks for the response Abe!
>     
>     Fair enough, I'm fine with having the functional tests committed soon after as this separate and new component. I'm assuming that the tests will be added soon after this get in, correct?
>     
>     The documentation JIRA SQOOP-1680 seems to be covering more an APIs for someone who is going to create new repository rather then instructing end user how to configure the repository. I know that all of the required properties are stored in "sqoop.properties" file and one just need to update them, but I was thinking about having a repository section in our admin guide that will list existing repositories and example configuration snipnets how to enable them.

Ah I understand. Definitely. Let me add docuemtation to the "Server installation" section of the docs.


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, lines 23-26
> > <https://reviews.apache.org/r/26948/diff/3/?file=779839#file779839line23>
> >
> >     I see the same very long comment in file PostgresqlSchemaCreateQuery. Let's perhaps have it in one single file and reference it in the other? I'm afraid that having it in several places will be harder for maintenance.
> 
> Abraham Elmahrek wrote:
>     https://issues.apache.org/jira/browse/SQOOP-1824
> 
> Jarek Cecho wrote:
>     I'm fine with having the schema inside java file, because it's internal to the repository implementation. End user (and admin) should never go directly to the database and do changes there. I'm just a bit worried that keeping the copy in two places will get easily out of sync.

Ah sorry. I misread this comment. There are two representations of the Schema in the postgresql package. Will rectify.


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.

Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.


- Abraham


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


On Dec. 4, 2014, 9:18 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2014, 9:18 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java

Please scratch 3), I found the clean up routine :)


- Jarek


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


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.
> 
> Abraham Elmahrek wrote:
>     Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.
> 
> Abraham Elmahrek wrote:
>     The PostgreSQL repository does work with upper case since it seems to force names to lower case. The difficulty is in metadata queries it seems. If we query the metadata, then it will differ. What do you think Jarcec? Should we make an exception for Metadata queries? Handle these issues on a case-by-case basis?
> 
> Jarek Cecho wrote:
>     If we want to continue reusing the shared code, then another solution would be to alter our queries to properly enclose the table names, e.g. instead of
>     
>     SELECT * FROM table
>     
>     We should use
>     
>     SELECT * FROM "table"
>     
>     The small issue is that the enclose characters are different for each database :)
>     
>     Those small caveheats and differences have been the reason why we've decided to have one repository implementation per target database rather then having generic JDBC layer like some of the other projects have.
> 
> Abraham Elmahrek wrote:
>     That sounds good. I believe the same structure exists for the "Provider" interface as well.
> 
> Abraham Elmahrek wrote:
>     Do you think we can do this in a separate Jira given PostgreSQL repository appears to be working?

The "Provider" is a test only though :)

I see your point with doing this post-commit, so I'm fine with resolving it in separate JIRA. Could you file one Abe?


- Jarek


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


On Dec. 18, 2014, 9:49 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2014, 9:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 609a875 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca590d8 
>   pom.xml aa4231e 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9fa2f9d 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 77477ee 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java
> 
> Jarek Cecho wrote:
>     Please scratch 3), I found the clean up routine :)
> 
> Jarek Cecho wrote:
>     I've give it a spin on my real cluster and I can confirm that it's working just fine, thank you Abe!
> 
> Abraham Elmahrek wrote:
>     Functional tests are in https://issues.apache.org/jira/browse/SQOOP-1591. Documentation is dependent on https://issues.apache.org/jira/browse/SQOOP-1680. Can create follow up on this though. Tests are removing entire schema in PostgresqlTestCase#tearDown. Is there any other clean up that you think might be useful?
> 
> Jarek Cecho wrote:
>     Thanks for the response Abe!
>     
>     Fair enough, I'm fine with having the functional tests committed soon after as this separate and new component. I'm assuming that the tests will be added soon after this get in, correct?
>     
>     The documentation JIRA SQOOP-1680 seems to be covering more an APIs for someone who is going to create new repository rather then instructing end user how to configure the repository. I know that all of the required properties are stored in "sqoop.properties" file and one just need to update them, but I was thinking about having a repository section in our admin guide that will list existing repositories and example configuration snipnets how to enable them.
> 
> Abraham Elmahrek wrote:
>     Ah I understand. Definitely. Let me add docuemtation to the "Server installation" section of the docs.

Actually Jarcec, looking closer at the Administrative documentation, I think that a new page that has content on all repository implementations would be useful. Would you mind if we separated this out to a separate Jira?


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.
> 
> Abraham Elmahrek wrote:
>     Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.

The PostgreSQL repository does work with upper case since it seems to force names to lower case. The difficulty is in metadata queries it seems. If we query the metadata, then it will differ. What do you think Jarcec? Should we make an exception for Metadata queries? Handle these issues on a case-by-case basis?


- Abraham


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


On Dec. 4, 2014, 9:18 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2014, 9:18 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.
> 
> Abraham Elmahrek wrote:
>     Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.
> 
> Abraham Elmahrek wrote:
>     The PostgreSQL repository does work with upper case since it seems to force names to lower case. The difficulty is in metadata queries it seems. If we query the metadata, then it will differ. What do you think Jarcec? Should we make an exception for Metadata queries? Handle these issues on a case-by-case basis?
> 
> Jarek Cecho wrote:
>     If we want to continue reusing the shared code, then another solution would be to alter our queries to properly enclose the table names, e.g. instead of
>     
>     SELECT * FROM table
>     
>     We should use
>     
>     SELECT * FROM "table"
>     
>     The small issue is that the enclose characters are different for each database :)
>     
>     Those small caveheats and differences have been the reason why we've decided to have one repository implementation per target database rather then having generic JDBC layer like some of the other projects have.
> 
> Abraham Elmahrek wrote:
>     That sounds good. I believe the same structure exists for the "Provider" interface as well.

Do you think we can do this in a separate Jira given PostgreSQL repository appears to be working?


- Abraham


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


On Dec. 10, 2014, 9:33 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 9:33 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e182176 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.
> 
> Abraham Elmahrek wrote:
>     Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.
> 
> Abraham Elmahrek wrote:
>     The PostgreSQL repository does work with upper case since it seems to force names to lower case. The difficulty is in metadata queries it seems. If we query the metadata, then it will differ. What do you think Jarcec? Should we make an exception for Metadata queries? Handle these issues on a case-by-case basis?
> 
> Jarek Cecho wrote:
>     If we want to continue reusing the shared code, then another solution would be to alter our queries to properly enclose the table names, e.g. instead of
>     
>     SELECT * FROM table
>     
>     We should use
>     
>     SELECT * FROM "table"
>     
>     The small issue is that the enclose characters are different for each database :)
>     
>     Those small caveheats and differences have been the reason why we've decided to have one repository implementation per target database rather then having generic JDBC layer like some of the other projects have.

That sounds good. I believe the same structure exists for the "Provider" interface as well.


- Abraham


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


On Dec. 10, 2014, 9:33 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 9:33 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e182176 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java
> 
> Jarek Cecho wrote:
>     Please scratch 3), I found the clean up routine :)

I've give it a spin on my real cluster and I can confirm that it's working just fine, thank you Abe!


- Jarek


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


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:
> > 
> > 1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]
> > 
> > 2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?
> > 
> > 3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.
> > 
> > Links:
> > 1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java
> 
> Jarek Cecho wrote:
>     Please scratch 3), I found the clean up routine :)
> 
> Jarek Cecho wrote:
>     I've give it a spin on my real cluster and I can confirm that it's working just fine, thank you Abe!
> 
> Abraham Elmahrek wrote:
>     Functional tests are in https://issues.apache.org/jira/browse/SQOOP-1591. Documentation is dependent on https://issues.apache.org/jira/browse/SQOOP-1680. Can create follow up on this though. Tests are removing entire schema in PostgresqlTestCase#tearDown. Is there any other clean up that you think might be useful?
> 
> Jarek Cecho wrote:
>     Thanks for the response Abe!
>     
>     Fair enough, I'm fine with having the functional tests committed soon after as this separate and new component. I'm assuming that the tests will be added soon after this get in, correct?
>     
>     The documentation JIRA SQOOP-1680 seems to be covering more an APIs for someone who is going to create new repository rather then instructing end user how to configure the repository. I know that all of the required properties are stored in "sqoop.properties" file and one just need to update them, but I was thinking about having a repository section in our admin guide that will list existing repositories and example configuration snipnets how to enable them.
> 
> Abraham Elmahrek wrote:
>     Ah I understand. Definitely. Let me add docuemtation to the "Server installation" section of the docs.
> 
> Abraham Elmahrek wrote:
>     Actually Jarcec, looking closer at the Administrative documentation, I think that a new page that has content on all repository implementations would be useful. Would you mind if we separated this out to a separate Jira?

Totally, created https://issues.apache.org/jira/browse/SQOOP-1892


> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)
> 
> Jarek Cecho wrote:
>     Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)
> 
> Abraham Elmahrek wrote:
>     In that case, for consistency, would you feel ok with having every thing lower case (table names, column names, etc.)?
> 
> Jarek Cecho wrote:
>     I honestly don't have strong preference whether the names should be lower or upper case. I just feel that we should be consistent in what we have in the constants and what is actually used in the database.
> 
> Abraham Elmahrek wrote:
>     Let me see what is possible. The "common" repository handler uses all uppercase. Postgresql should have no difficulty reading those names I think.
> 
> Abraham Elmahrek wrote:
>     The PostgreSQL repository does work with upper case since it seems to force names to lower case. The difficulty is in metadata queries it seems. If we query the metadata, then it will differ. What do you think Jarcec? Should we make an exception for Metadata queries? Handle these issues on a case-by-case basis?

If we want to continue reusing the shared code, then another solution would be to alter our queries to properly enclose the table names, e.g. instead of

SELECT * FROM table

We should use

SELECT * FROM "table"

The small issue is that the enclose characters are different for each database :)

Those small caveheats and differences have been the reason why we've decided to have one repository implementation per target database rather then having generic JDBC layer like some of the other projects have.


- Jarek


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


On Dec. 10, 2014, 9:33 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 9:33 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e182176 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.

> On Dec. 4, 2014, 6:30 p.m., Jarek Cecho wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 83-84
> > <https://reviews.apache.org/r/26948/diff/3/?file=779836#file779836line83>
> >
> >     If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)

Actually, I would generalize my comment. When playing with the PostgreSQL repository on real cluster, I've noticed that PostgreSQL will lower case all table/column names that are not explicilty quoted. Hence I would suggest to either lower case all the constants or escape them in the queries - whatever we prefer more :)


- Jarek


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


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/#review63855
-----------------------------------------------------------


Good job Abe! I've just read the code and I'm still yet to try the changes on real cluster, but I wanted to share my thoughts early. I have couple of high level questions and then couple of nits:

1) Are you also planning to provide functional tests in addition to the structural ones? Like create link/job and verifying other calls that are prescribed by Repository interface [1]

2) Are you planning to provide a documentation on how to use the new repository? Perhaps a section in the Administration guide talking about different repository implementations?

3) The tests are creating a lot of tables, but I did not see any clean up. In case that I've did not simply missed it, we should add it, otherwise test re-execution won't be that simple.

Links:
1: https://github.com/apache/sqoop/blob/sqoop2/core/src/main/java/org/apache/sqoop/repository/Repository.java


repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment106160>

    Future: Perhaps a suggestion for future, I think that this can be further improved to simply use PrepareStatement.setObject() to support any type :)



repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java
<https://reviews.apache.org/r/26948/#comment106161>

    Nit: Can we do this change in separate JIRA? Cleaning up Derby repository structures as part of "Introduce PostgreSQL repository" doesn't seem right :)



repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment106162>

    Nit: Similarly for this method re-order, let's not do it as part of introducing PostgreSQL repository.



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
<https://reviews.apache.org/r/26948/#comment106163>

    We don't have to be on equal version with Derby - this is clean start, so let's start with 1?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
<https://reviews.apache.org/r/26948/#comment106164>

    Super nit: Let's put comma after the final one, so that adding new error code doesn't require changing this line.



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment106165>

    If we want to use the constant in Lower case, then let's define them as such? It seems weird to have the contant in upper case and then lowercase it on every use :)



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment106166>

    I'm assuming that those runQuery(*_CREATE_*) should be inside the if (version == 0 ) statement?
    
    Otherwise we will try to create the tables on every method execution which I assume will always fail on the cond try, right?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment106167>

    Since we are throwing exception here, let's use LOG.error or LOG.fatal intead of LOG.warn? :)



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment106169>

    Let's change the comment to valid "PostgreSQL" Query? :)



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java
<https://reviews.apache.org/r/26948/#comment106176>

    What about making reasonable exception in our "no asterisks imports" rule here :)



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
<https://reviews.apache.org/r/26948/#comment106177>

    I see the same very long comment in file PostgresqlSchemaCreateQuery. Let's perhaps have it in one single file and reference it in the other? I'm afraid that having it in several places will be harder for maintenance.



repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java
<https://reviews.apache.org/r/26948/#comment106178>

    Let's not however use asterisk import for java.util package classes.



repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java
<https://reviews.apache.org/r/26948/#comment106179>

    Let's perhaps call those statements "assertTableExists", it seems that that is the way we are using in other assert-like methods.



repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
<https://reviews.apache.org/r/26948/#comment106181>

    The provider is loading the driver on the start method already:
    
    https://github.com/apache/sqoop/blob/sqoop2/common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java#L126



repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
<https://reviews.apache.org/r/26948/#comment106190>

    I'm a bit worried about this approach as it will hide configuration issues - e.g. if the database is down or credentials are missconfigured, the test will still pass and the fact that we are not testing PostgreSQL repo will be silently ignored.
    
    Let's perhaps promote those tests into Integration tests similarly as we're doing in test module [1] and add a flag to enable them?
    
    Another idea would be to use JUnit categories [2] and it's maven integration to run them in a special profile/flag.
    
    I'm happy to take this one into subsequent JIRA as it's more general topic though.
    
    Links:
    1: https://github.com/apache/sqoop/blob/sqoop2/test/pom.xml#L154
    2: https://weblogs.java.net/blog/johnsmart/archive/2010/04/25/grouping-tests-using-junit-categories-0
    3: http://maven.apache.org/surefire/maven-surefire-plugin/examples/junit.html



repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
<https://reviews.apache.org/r/26948/#comment106182>

    I'm wondering why the entire package name in the call here - it seems that we are importing other Junit related classes.


Jarcec

- Jarek Cecho


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/#review65898
-----------------------------------------------------------

Ship it!


All my comments have been either incoporated or moved to a subsequent JIRA, so I'm +1 on the patch. Good job Abe!

One note - it seems that the patch no longer works correctly with some recent changes in our pom files:

[ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.sqoop:sqoop-common-test:2.0.0-SNAPSHOT'}' and 'Vertex{label='org.apache.sqoop:sqoop-common:2.0.0-SNAPSHOT'}' introduces to cycle in the graph org.apache.sqoop:sqoop-common:2.0.0-SNAPSHOT --> org.apache.sqoop:sqoop-common-test:2.0.0-SNAPSHOT --> org.apache.sqoop:sqoop-common:2.0.0-SNAPSHOT -> [Help 1]

Would you mind addressing this one? I'll commit it after that.

- Jarek Cecho


On Dec. 18, 2014, 9:49 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2014, 9:49 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 609a875 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca590d8 
>   pom.xml aa4231e 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9fa2f9d 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 77477ee 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/
-----------------------------------------------------------

(Updated Dec. 18, 2014, 9:49 p.m.)


Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs (updated)
-----

  common-test/pom.xml 609a875 
  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca590d8 
  pom.xml aa4231e 
  repository/pom.xml 8c95c0e 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9fa2f9d 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 77477ee 

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


Testing
-------

Can start Sqoop2 server.
Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
Also:
 482  disable link -l 1
 483  show link --all
 484  show link
 485  enable link -l 1
 486  show link
 487  show link --all
 488  show job
 489  show job --all
 490  show connector
 491  show connector --all
 492  help
 493  status job --jid 1
 494  history
 495  start job --jid 1
 496  clone job --jid 1
 497  delete job --jid 2
 498  status job --jid 1
 499  history


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/
-----------------------------------------------------------

(Updated Dec. 10, 2014, 9:33 p.m.)


Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs (updated)
-----

  common-test/pom.xml 9fd671c 
  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
  pom.xml e182176 
  repository/pom.xml 8c95c0e 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 1adcca0 

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


Testing
-------

Can start Sqoop2 server.
Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
Also:
 482  disable link -l 1
 483  show link --all
 484  show link
 485  enable link -l 1
 486  show link
 487  show link --all
 488  show job
 489  show job --all
 490  show connector
 491  show connector --all
 492  help
 493  status job --jid 1
 494  history
 495  start job --jid 1
 496  clone job --jid 1
 497  delete job --jid 2
 498  status job --jid 1
 499  history


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/
-----------------------------------------------------------

(Updated Dec. 4, 2014, 9:18 p.m.)


Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs (updated)
-----

  common-test/pom.xml 9fd671c 
  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
  pom.xml e6ffc78 
  repository/pom.xml 8c95c0e 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 1adcca0 

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


Testing
-------

Can start Sqoop2 server.
Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
Also:
 482  disable link -l 1
 483  show link --all
 484  show link
 485  enable link -l 1
 486  show link
 487  show link --all
 488  show job
 489  show job --all
 490  show connector
 491  show connector --all
 492  help
 493  status job --jid 1
 494  history
 495  start job --jid 1
 496  clone job --jid 1
 497  delete job --jid 2
 498  status job --jid 1
 499  history


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/
-----------------------------------------------------------

(Updated Dec. 2, 2014, 1:48 a.m.)


Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs (updated)
-----

  common-test/pom.xml 9fd671c 
  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
  common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
  pom.xml e6ffc78 
  repository/pom.xml 8c95c0e 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
  repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
  repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 1adcca0 

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


Testing
-------

Can start Sqoop2 server.
Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
Also:
 482  disable link -l 1
 483  show link --all
 484  show link
 485  enable link -l 1
 486  show link
 487  show link --all
 488  show job
 489  show job --all
 490  show connector
 491  show connector --all
 492  help
 493  status job --jid 1
 494  history
 495  start job --jid 1
 496  clone job --jid 1
 497  delete job --jid 2
 498  status job --jid 1
 499  history


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Qian Xu <sx...@googlemail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/#review61701
-----------------------------------------------------------



repository/repository-postgresql/pom.xml
<https://reviews.apache.org/r/26948/#comment103514>

    If only for testing, then considering add `<scope>test</scope>`,



common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java
<https://reviews.apache.org/r/26948/#comment103512>

    NITS: localhost can be omitted. Use `jdbc:postgresql:/sqoop`



common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java
<https://reviews.apache.org/r/26948/#comment103513>

    If esacpe is not required/allowed here, better leave a comment for code readers.


- Qian Xu


On Nov. 12, 2014, 4:25 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2014, 4:25 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml 9ff51d1 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9f4ecbc 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 769544b 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 4d99a52 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 4a5eb5e 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/
-----------------------------------------------------------

(Updated Nov. 12, 2014, 8:25 a.m.)


Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs
-----

  common-test/pom.xml 9fd671c 
  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
  common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
  pom.xml 9ff51d1 
  repository/pom.xml 8c95c0e 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9f4ecbc 
  repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 769544b 
  repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 4d99a52 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 4a5eb5e 

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


Testing (updated)
-------

Can start Sqoop2 server.
Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
Also:
 482  disable link -l 1
 483  show link --all
 484  show link
 485  enable link -l 1
 486  show link
 487  show link --all
 488  show job
 489  show job --all
 490  show connector
 491  show connector --all
 492  help
 493  status job --jid 1
 494  history
 495  start job --jid 1
 496  clone job --jid 1
 497  delete job --jid 2
 498  status job --jid 1
 499  history


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/
-----------------------------------------------------------

(Updated Nov. 12, 2014, 8:23 a.m.)


Review request for Sqoop.


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


Repository: sqoop-sqoop2


Description
-------

commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date:   Tue Oct 14 12:05:53 2014 -0700

    Sqoop2: Postgresql repository

:100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
:100644 100644 f25a29f... 5be6ad9... M  pom.xml
:100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
:000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
:000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
:000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
:000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
:000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
:000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
:000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
:000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
:000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
:100644 100644 67baaa5... 8ecdd01... M  server/pom.xml


Diffs (updated)
-----

  common-test/pom.xml 9fd671c 
  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
  common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
  pom.xml 9ff51d1 
  repository/pom.xml 8c95c0e 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java 9f4ecbc 
  repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 769544b 
  repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 4d99a52 
  repository/repository-postgresql/pom.xml PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
  repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
  server/pom.xml 4a5eb5e 

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


Testing
-------

Can start Sqoop2 server.


Thanks,

Abraham Elmahrek


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Oct. 21, 2014, 12:55 a.m., Gwen Shapira wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 349-352
> > <https://reviews.apache.org/r/26948/diff/1/?file=726086#file726086line349>
> >
> >     Can this actually happen? we are only adding PG now... if this is just in case, maybe write something to the log so we'll know something weird happened?
> 
> Abraham Elmahrek wrote:
>     It can actually happen: version = 0 when we're starting from scratch. Otherwise, version should be 4. Logging makes sense and throwing an exception makes sense. This really should only happen if someone transfers databases via SQL or something.

Actually, a second thought... we should probably keep the casing in order to re-use common repository functionality.


- Abraham


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


On Dec. 2, 2014, 1:48 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 1:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   common-test/pom.xml 9fd671c 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 0a12d7b 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java d46e01d 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 4fe1500 
>   pom.xml e6ffc78 
>   repository/pom.xml 8c95c0e 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java c278406 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 6bc5674 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 907978f 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgreSQLTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 1adcca0 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> Ran integration tests with PostgreSQL. All pass except partitioning on a boolean column.
> Also:
>  482  disable link -l 1
>  483  show link --all
>  484  show link
>  485  enable link -l 1
>  486  show link
>  487  show link --all
>  488  show job
>  489  show job --all
>  490  show connector
>  491  show connector --all
>  492  help
>  493  status job --jid 1
>  494  history
>  495  start job --jid 1
>  496  clone job --jid 1
>  497  delete job --jid 2
>  498  status job --jid 1
>  499  history
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Oct. 21, 2014, 12:55 a.m., Gwen Shapira wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java, lines 349-352
> > <https://reviews.apache.org/r/26948/diff/1/?file=726086#file726086line349>
> >
> >     Can this actually happen? we are only adding PG now... if this is just in case, maybe write something to the log so we'll know something weird happened?

It can actually happen: version = 0 when we're starting from scratch. Otherwise, version should be 4. Logging makes sense and throwing an exception makes sense. This really should only happen if someone transfers databases via SQL or something.


> On Oct. 21, 2014, 12:55 a.m., Gwen Shapira wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, line 237
> > <https://reviews.apache.org/r/26948/diff/1/?file=726088#file726088line237>
> >
> >     Did you validate that SYS.SYSSCHEMAS exist in PG?  
> >     I think you need:
> >     select oid from pg_catalog.pg_namespace where nspname=...
> >     
> >     Or maybe something from information_schema.

There's a metadata call in PostgresqlRepositoryHandler#detectVersion.


> On Oct. 21, 2014, 12:55 a.m., Gwen Shapira wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, line 333
> > <https://reviews.apache.org/r/26948/diff/1/?file=726088#file726088line333>
> >
> >     Do you think we want to support CLOBs? Its a pain for the db admins. Perhaps limit config values to 4000 characters?

Maybe 1000 characters? Might be cool to make room for 4-byte unicode.


> On Oct. 21, 2014, 12:55 a.m., Gwen Shapira wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, line 490
> > <https://reviews.apache.org/r/26948/diff/1/?file=726088#file726088line490>
> >
> >     Again, I think you don't need this condition - you already filtered on SQBI_JOB

Moved to common construct. Can fix in diff. jira.


> On Oct. 21, 2014, 12:55 a.m., Gwen Shapira wrote:
> > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java, line 471
> > <https://reviews.apache.org/r/26948/diff/1/?file=726088#file726088line471>
> >
> >     I think you can skip this line since you are already filtering for the right link in the join condition.
> >     
> >     predicates of the form ( X=Y OR X IS NULL) pretty much force full table scans (since index won't contain null), so I like avoiding them. Not always possible, but I think its possible here.

Moved to common construct. Can fix in diff. jira.


On Oct. 21, 2014, 12:55 a.m., Abraham Elmahrek wrote:
> > As we discussed, I think we need a preceeding JIRA to move all the reusable methods from DerbyHandler to JdbcHandler so we won't need to maintain them for each DB. It should also make this one easier to review since I'll be able to see what is actually new :)

Yeah, that works. Let's do it in https://issues.apache.org/jira/browse/SQOOP-1589.


- Abraham


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


On Oct. 20, 2014, 6:47 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2014, 6:47 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5a8e026 
>   pom.xml f25a29f 
>   repository/pom.xml e3345c4 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 67baaa5 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/#review57474
-----------------------------------------------------------



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
<https://reviews.apache.org/r/26948/#comment98189>

    Looks like it is identical to Derby implementation. Maybe pull to a superclass things that will be the same for all repositories?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
<https://reviews.apache.org/r/26948/#comment98190>

    Many errors will be identical in all repository implementations. Maybe we can pull this to a superclass?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment98191>

    Wondering if this can be made concrete in JdbcRepositoryHandler. Its not specific to Derby or Postgres at all.
    
    Actually, a lot of the code here isn't. Any reason we are not moving this up?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment98196>

    Can this actually happen? we are only adding PG now... if this is just in case, maybe write something to the log so we'll know something weird happened?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
<https://reviews.apache.org/r/26948/#comment98202>

    This is a separate JIRA, right? Maybe just open it and remove the todo? It will have to be done for both implementations (or just on JdbcHandler) anyway.



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
<https://reviews.apache.org/r/26948/#comment98218>

    Did you validate that SYS.SYSSCHEMAS exist in PG?  
    I think you need:
    select oid from pg_catalog.pg_namespace where nspname=...
    
    Or maybe something from information_schema.



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
<https://reviews.apache.org/r/26948/#comment98219>

    Nice! didn't know about bigserial.



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
<https://reviews.apache.org/r/26948/#comment98221>

    Do you think we want to support CLOBs? Its a pain for the db admins. Perhaps limit config values to 4000 characters?



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
<https://reviews.apache.org/r/26948/#comment98222>

    I think you can skip this line since you are already filtering for the right link in the join condition.
    
    predicates of the form ( X=Y OR X IS NULL) pretty much force full table scans (since index won't contain null), so I like avoiding them. Not always possible, but I think its possible here.



repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
<https://reviews.apache.org/r/26948/#comment98223>

    Again, I think you don't need this condition - you already filtered on SQBI_JOB


As we discussed, I think we need a preceeding JIRA to move all the reusable methods from DerbyHandler to JdbcHandler so we won't need to maintain them for each DB. It should also make this one easier to review since I'll be able to see what is actually new :)

- Gwen Shapira


On Oct. 20, 2014, 6:47 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2014, 6:47 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5a8e026 
>   pom.xml f25a29f 
>   repository/pom.xml e3345c4 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 67baaa5 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository

Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26948/#review57728
-----------------------------------------------------------


Most of my comments are inline with Gwen. 

In my very sweet words this RB has atleast 50% of the exact same things Derby code has. Lets not create more tech debt with this c&p. Pull a lot of things out into common classes or even Utils.

Even the test methods can be shared, kindly spend time extracting it out to a utils code

Also, maintaining the sql scripts in code is a pain especially with upgrade, is there a way to avoid this going forward. If we need ideas, a simple brain dead solution is to have versioned sql scripts that are invoked from code.

- Veena Basavaraj


On Oct. 20, 2014, 11:47 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26948/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2014, 11:47 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1523
>     https://issues.apache.org/jira/browse/SQOOP-1523
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 0702bddfa69f63f56bfc6909dfe1b3e4f3219c6a
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date:   Tue Oct 14 12:05:53 2014 -0700
> 
>     Sqoop2: Postgresql repository
> 
> :100644 100644 5a8e026... 2c5c972... M  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> :100644 100644 f25a29f... 5be6ad9... M  pom.xml
> :100644 100644 e3345c4... 8b308c9... M  repository/pom.xml
> :000000 100644 0000000... 19f67d4... A  repository/repository-postgresql/pom.xml
> :000000 100644 0000000... cdeed35... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java
> :000000 100644 0000000... 872683e... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java
> :000000 100644 0000000... 93352a6... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java
> :000000 100644 0000000... d1ff898... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java
> :000000 100644 0000000... d70dd86... A  repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java
> :000000 100644 0000000... 31cc6dc... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java
> :000000 100644 0000000... a8f4242... A  repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java
> :000000 100644 0000000... 44ffced... A  repository/repository-postgresql/src/test/resources/log4j.properties
> :100644 100644 67baaa5... 8ecdd01... M  server/pom.xml
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 5a8e026 
>   pom.xml f25a29f 
>   repository/pom.xml e3345c4 
>   repository/repository-postgresql/pom.xml PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoError.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaConstants.java PRE-CREATION 
>   repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlSchemaQuery.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java PRE-CREATION 
>   repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-postgresql/src/test/resources/log4j.properties PRE-CREATION 
>   server/pom.xml 67baaa5 
> 
> Diff: https://reviews.apache.org/r/26948/diff/
> 
> 
> Testing
> -------
> 
> Can start Sqoop2 server.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>