> 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 <[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 > >
