> On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java, > > lines 979-982 > > <https://reviews.apache.org/r/24957/diff/1/?file=669690#file669690line979> > > > > Do you also add FK for "FROM" connection?
The constraint exists since we alter the column name SQN_CONNECTION to SQN_FROM_CONNECTION. Seems OK to rename the constraint. > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java, > > line 970 > > <https://reviews.apache.org/r/24957/diff/1/?file=669690#file669690line970> > > > > Isn't this actually version 4 upgrade? (i.e - from any version to > > version 4) Upgrading from version 3 to version 4. Will change! > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java, > > lines 291-299 > > <https://reviews.apache.org/r/24957/diff/1/?file=669690#file669690line291> > > > > Now I'm confused. Jobs have two connections and no type... didn't you > > just do the reverse here? Jobs have a FROM connection and a TO connection. There should be no sense of TYPE since TYPE was used to define IMPORT / EXPORT. Instead, we just want to transfer data FROM one location TO another location. > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java, > > lines 109-110 > > <https://reviews.apache.org/r/24957/diff/1/?file=669690#file669690line109> > > > > you need FK on both, right? It's there, updating docs. > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, > > lines 406-407 > > <https://reviews.apache.org/r/24957/diff/1/?file=669688#file669688line406> > > > > I don't get it. Don't we know that this is a non-fresh install because > > version<=3? Fresh install will have version=4, right? Fresh installs go through the entire upgrade path with no data. > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, > > line 399 > > <https://reviews.apache.org/r/24957/diff/1/?file=669688#file669688line399> > > > > This is in the DerbyRepositoryHandler, but has no dependency on Derby > > at all. Can we pull it out so we won't need to copy-paste it into all the > > repository implementations? Actually, I don't know how much of this is specific to Derby or not. Let's pull it out in a separate Jira since I really have no idea what I will break. > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, > > lines 527-529 > > <https://reviews.apache.org/r/24957/diff/1/?file=669688#file669688line527> > > > > I find hardcoded column numbers difficult to read. can we use constants? I don't think we should create constants for indices since this can lead to several constants whenever we change schemas or how we query the database. I can add comments? > On Aug. 26, 2014, 11:53 p.m., Gwen Shapira wrote: > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java, > > line 444 > > <https://reviews.apache.org/r/24957/diff/1/?file=669688#file669688line444> > > > > Same as above, this has nothing to do with Derby at all... can we move > > it out? Separate Jira for same reasons as above. - Abraham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24957/#review51609 ----------------------------------------------------------- On Aug. 26, 2014, 10:16 p.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24957/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2014, 10:16 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1374 > https://issues.apache.org/jira/browse/SQOOP-1374 > > > Repository: sqoop-SQOOP-1367 > > > Description > ------- > > commit 3e4abe598f2df1d109a4401f0961b91f229b0be1 > Author: Abraham Elmahrek <[email protected]> > Date: Wed Aug 6 15:26:39 2014 -0700 > > SQOOP-1374: From/To: Metadata upgrade > > :100644 100644 d9666c8... a5399fd... M > common/src/main/java/org/apache/sqoop/model/FormUtils.java > :100644 100644 e5a4fb8... 8336fb7... M > common/src/main/java/org/apache/sqoop/model/MConnection.java > :100644 100644 11839fc... 6d3fe4b... M > common/src/main/java/org/apache/sqoop/model/MJob.java > :100644 100644 2b12009... cbe72f6... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java > :100644 100644 eea86b2... 5447623... M > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java > :100644 100644 557091e... 883636c... M > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > :000000 100644 0000000... 3e51e38... A > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsMetadataUpgrader.java > :100644 100644 b92ff4d... db6f579... M > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java > :000000 100644 0000000... 4b93c2a... A > core/src/main/java/org/apache/sqoop/connector/ConnectorManagerUtils.java > :100644 100644 9b64661... fa119a5... M > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java > :100644 100644 e9c32e0... f75b5b3... M > core/src/main/java/org/apache/sqoop/repository/Repository.java > :100644 100644 030dde7... 41c9bcd... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > :100644 100644 88be9fb... 1f930a6... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > :100644 100644 1a77360... 1f030ea... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > :100644 100644 e5bb2e7... 1903b26... M > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > :100644 100644 f603cc1... 4f0b139... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > :100644 100644 bdd3c05... f9e9217... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectionHandling.java > :100644 100644 54ae726... 745e128... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > :100644 100644 8b3326d... 006ec9c... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestFrameworkHandling.java > :100644 100644 5d3807d... 15f9539... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > :100644 100644 0d93348... 9282c0c... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInternals.java > :100644 100644 2260a45... 02a39be... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java > :100644 100644 8cfe076... 8fce0dd... M > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java > :100644 100644 d840a78... 328f9b0... M > spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/model/FormUtils.java d9666c8 > common/src/main/java/org/apache/sqoop/model/MConnection.java e5a4fb8 > common/src/main/java/org/apache/sqoop/model/MJob.java 11839fc > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java > 2b12009 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcValidator.java > eea86b2 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java > 557091e > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsMetadataUpgrader.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java b92ff4d > core/src/main/java/org/apache/sqoop/connector/ConnectorManagerUtils.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 9b64661 > core/src/main/java/org/apache/sqoop/repository/Repository.java e9c32e0 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoConstants.java > 030dde7 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 88be9fb > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaConstants.java > 1a77360 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java > e5bb2e7 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java > f603cc1 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectionHandling.java > bdd3c05 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestConnectorHandling.java > 54ae726 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestFrameworkHandling.java > 8b3326d > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > 5d3807d > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInternals.java > 0d93348 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java > 2260a45 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java > 8cfe076 > spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java > d840a78 > > Diff: https://reviews.apache.org/r/24957/diff/ > > > Testing > ------- > > Ran derby repository tests manually. > > > Thanks, > > Abraham Elmahrek > >
