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