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