----------------------------------------------------------- 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 <[email protected]> > 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 > >
