sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 36918: SQOOP-2244 Sqoop2: Generic JDBC: Automatically escape table and column names from configuration objects
Date Thu, 30 Jul 2015 15:19:58 GMT


> On July 30, 2015, 1:41 a.m., Abraham Elmahrek wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java,
line 121
> > <https://reviews.apache.org/r/36918/diff/1/?file=1024665#file1024665line121>
> >
> >     Default can easily vary based on JDBC url as well. What do you think?

Good point, I should have provide more details.

SQL spec specifies that the default escape character is '"'. I've verified MySQL, Oracle,
PostgreSQL and Microsoft SQL Server and they all work correctly with '"' even though thay
have their own "natural" escape characters. The only issue is that for MySQL you need to set
SQL mode of ANSI_QUOTES [1]. That is also why I've made it configurable, but it will work
by default for any database that is following SQL spec which seems fair for Generic JDBC connector
:)

Links:
1: https://dev.mysql.com/doc/refman/5.0/en/sql-mode.html#sqlmode_ansi_quotes


> On July 30, 2015, 1:41 a.m., Abraham Elmahrek wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java,
line 163
> > <https://reviews.apache.org/r/36918/diff/1/?file=1024665#file1024665line163>
> >
> >     Seems like quoting versus escaping.

That is a good point, I'll rename the method.


> On July 30, 2015, 1:41 a.m., Abraham Elmahrek wrote:
> > connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties,
line 130
> > <https://reviews.apache.org/r/36918/diff/1/?file=1024670#file1024670line130>
> >
> >     Doesn't seem like escaping per-say, but quoting?

That is a good point, I'll rename the method.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36918/#review93537
-----------------------------------------------------------


On July 29, 2015, 5:11 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36918/
> -----------------------------------------------------------
> 
> (Updated July 29, 2015, 5:11 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2244
>     https://issues.apache.org/jira/browse/SQOOP-2244
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The patch is rather big as there has been a lot of things that I had to change. I've
changed the semantics on the configuration objects now - we're expecting unescaped table/column
names and we're doing the escaping whenever it's needed.
> 
> 
> Diffs
> -----
> 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
cab0917 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java
20fabf6 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
4688de3 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/LinkConfiguration.java
ed55bff 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/SqlDialect.java
PRE-CREATION 
>   connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties
52bf631 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java
22c9e15 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcTestConstants.java
8a5dba4 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java
77ac9c3 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java
6ae6f90 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java
f192c22 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java
1c65fc3 
>   test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java c84e799 
>   test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java
dac6db7 
>   test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/IncrementalReadTest.java
66c016d 
>   test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableStagedRDBMSTest.java
e9c4543 
> 
> Diff: https://reviews.apache.org/r/36918/diff/
> 
> 
> Testing
> -------
> 
> Both unit and integration tests are passing.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


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