sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Abraham Elmahrek" <...@cloudera.com>
Subject Re: Review Request 26948: SQOOP-1523: Sqoop2: Support for PostgreSQL database for hosting the metadata repository
Date Sat, 13 Dec 2014 17:43:51 GMT


> 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 <abraham@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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message