> On Jan. 15, 2015, 3:53 p.m., Jarek Cecho wrote: > > common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java, > > line 67 > > <https://reviews.apache.org/r/29749/diff/4/?file=819457#file819457line67> > > > > I'm wondering what is the reason to drop the proper escaping? > > Abraham Elmahrek wrote: > PostgreSQL escaping in a nut shell: > http://stackoverflow.com/questions/6331504/omitting-the-double-quote-to-do-query-on-postgresql. > > Essentially, we create tables without any quoting so that the common > repository handler will work. By adding quotes after the fact, it's difficult > to get the correct query. It's much more reasonable to leave the quotes out. > > Jarek Cecho wrote: > I do see why we would want to stop escaping the table names because we > are having inconsistencies in PostgreSQL repository. However I strongly > believe that this is a bug in the Repository implementation and it should be > fixed there rather then making the strict test provider more relaxed. > Otherwise we will very sooon end up having all sort of random errors such as > those that we've recently fixed in Sqoop 1 (SQOOP-1890). E.g I think that we > should properly escape all table names rather then the other way around.
Let's fix in a separate bug? This requires significant changes that's far outside the scope of this jira. - Abraham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29749/#review68259 ----------------------------------------------------------- On Jan. 16, 2015, 11:32 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29749/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2015, 11:32 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1591 > https://issues.apache.org/jira/browse/SQOOP-1591 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 560387f1d3e6869ada914fa6ae5ada7d8e210721 > Author: Abraham Elmahrek <[email protected]> > Date: Wed Jan 7 20:44:44 2015 -0800 > > SQOOP-1591: Sqoop2: PostgreSQL integration tests > > :100644 100644 8196fe2... cb987f6... M > common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java > :100644 100644 82289e8... 3a6205e... M > common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java > :100644 100644 5d80dce... 7248d7c... M > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java > :000000 100644 0000000... 85f895d... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestConnectorHandling.java > :000000 100644 0000000... 6022bb3... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestDriverHandling.java > :000000 100644 0000000... 1dfacb5... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestJobHandling.java > :000000 100644 0000000... 54c598e... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestLinkHandling.java > :100644 100644 2da19bc... 4735240... M > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java > :000000 100644 0000000... 9db7940... A > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestSubmissionHandling.java > > > Diffs > ----- > > > common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java > fb4e7af > > common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java > 82289e8 > > common-test/src/main/java/org/apache/sqoop/common/test/db/PostgreSQLProvider.java > d46e01d > repository/repository-postgresql/pom.xml 0ee9081 > > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepoConstants.java > bdefd4c > > repository/repository-postgresql/src/main/java/org/apache/sqoop/repository/postgresql/PostgresqlRepositoryHandler.java > 4013d22 > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/PostgresqlTestCase.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/PostgresqlTestUtils.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestConnectorHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestDriverHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestLinkHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestStructure.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestSubmissionHandling.java > PRE-CREATION > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestCase.java > ae546f3 > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/PostgresqlTestUtils.java > f393521 > > repository/repository-postgresql/src/test/java/org/apache/sqoop/repository/postgresql/TestStructure.java > 6075de4 > > Diff: https://reviews.apache.org/r/29749/diff/ > > > Testing > ------- > > mvn clean test -pl repository/repository-postgresql > -Dsqoop.provider.class=org.apache.sqoop.common.test.db.PostgreSQLProvider > -Dsqoop.provider.postgresql.jdbc=jdbc:postgresql://.../test > -Dsqoop.provider.postgresql.username=test > -Dsqoop.provider.postgresql.password=test > > > Thanks, > > Abraham Elmahrek > >
