> On Nov. 20, 2015, 7:10 p.m., Jarek Cecho wrote: > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, > > lines 379-380 > > <https://reviews.apache.org/r/40522/diff/2/?file=1133763#file1133763line379> > > > > Can't we just change the query to join with the connector table and > > search by the connector name directly? > > > > Seems unnecessary to construct the whole MConnector structure here.
This is an INSERT statement, and we have to get the connector id by the SQL. I agree that whole MConnector is unnecessary here. To get connectorId only, there should be a new SQL like: select connectorId from connectorTable where connectorName = XXXX. What do you think? reuse the current heavy function or create a new simple SQL and function? If we need create a new SQL, how about create a new JIRA for it? > On Nov. 20, 2015, 7:10 p.m., Jarek Cecho wrote: > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, > > line 1542 > > <https://reviews.apache.org/r/40522/diff/2/?file=1133763#file1133763line1542> > > > > Can we change the queries to resolve the connector name (additinal > > join)? Good catch, check the sql again, the connector name is avaiable already. I'll update it. > On Nov. 20, 2015, 7:10 p.m., Jarek Cecho wrote: > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java, > > line 1587 > > <https://reviews.apache.org/r/40522/diff/2/?file=1133763#file1133763line1587> > > > > Can we change the queries to resolve the connector name (additinal > > join)? This will be updated. > On Nov. 20, 2015, 7:10 p.m., Jarek Cecho wrote: > > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/AppendModeTest.java, > > lines 49-51 > > <https://reviews.apache.org/r/40522/diff/2/?file=1133784#file1133784line49> > > > > Could you describe why do we need to do the manual clear here? We're > > creating new MJob, so the values should be empty by default. Are we just > > missing calling .clone(false) somewhere? Check the again, currently, we get the MFromConfig, MToConfig from cache in SqoopClient. The SqoopClient is shared in all test cases under one suite. Some test cases get the MFromConfig/MToConfig, change it manual and save it back to cache again, for the next test case, the MFromConfig/MToConfig will be invalid. Clear the cache after every test case can fix it, will drop the current implementation for this problem. Thanks for reminder this. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40522/#review107394 ----------------------------------------------------------- On Nov. 23, 2015, 4:40 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40522/ > ----------------------------------------------------------- > > (Updated Nov. 23, 2015, 4:40 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Currently connector id is used in MLink as a foreigner key, the connector id > will be removed from public API, this should be updated as connector name. > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java 3d3425d > > client/src/main/java/org/apache/sqoop/client/request/ConnectorResourceRequest.java > f23c6e4 > > client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java > 0987703 > client/src/test/java/org/apache/sqoop/client/TestSqoopClient.java c962358 > common/src/main/java/org/apache/sqoop/json/ConnectorBean.java 89ae20c > common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java 9762ffa > common/src/main/java/org/apache/sqoop/json/LinkBean.java c9a8ab3 > common/src/main/java/org/apache/sqoop/model/MLink.java 51a4008 > common/src/test/java/org/apache/sqoop/json/TestConnectorBean.java a978d62 > common/src/test/java/org/apache/sqoop/json/TestConnectorsBean.java 15377c9 > common/src/test/java/org/apache/sqoop/json/util/BeanTestUtil.java 51ab0e5 > common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java > 5bf2465 > common/src/test/java/org/apache/sqoop/model/TestMLink.java b5df18a > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java db626a1 > core/src/main/java/org/apache/sqoop/driver/JobManager.java 90ee541 > core/src/test/java/org/apache/sqoop/driver/TestJobManager.java f137eef > core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java > 00b0511 > > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java > d1ee320 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestInputTypes.java > 193ee5f > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestLinkHandling.java > 92d1fae > > repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java > 232ef4c > > repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/PostgresqlTestCase.java > b8b0f52 > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > 52abe72 > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 9e75258 > shell/src/main/java/org/apache/sqoop/shell/CloneJobFunction.java 15cfad7 > shell/src/main/java/org/apache/sqoop/shell/CloneLinkFunction.java a679be8 > shell/src/main/java/org/apache/sqoop/shell/CreateJobFunction.java 79caa0d > shell/src/main/java/org/apache/sqoop/shell/CreateLinkFunction.java 289c3c3 > shell/src/main/java/org/apache/sqoop/shell/ShowConnectorFunction.java > 6efb51c > shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java 21873cc > shell/src/main/java/org/apache/sqoop/shell/ShowLinkFunction.java 04dd228 > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java 49cfd0b > shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 1bb7cd5 > shell/src/test/java/org/apache/sqoop/shell/TestCloneCommand.java eed27d7 > shell/src/test/java/org/apache/sqoop/shell/TestCreateCommand.java 48f646b > shell/src/test/java/org/apache/sqoop/shell/TestShowCommand.java 4272386 > shell/src/test/java/org/apache/sqoop/shell/TestUpdateCommand.java 9b3e87a > test/src/main/java/org/apache/sqoop/test/testcases/JettyTestCase.java > bd4ba6a > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java > 7fec310 > > test/src/test/java/org/apache/sqoop/integration/server/ShowJobInOrderTest.java > cbf1e90 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java > 75d2182 > > Diff: https://reviews.apache.org/r/40522/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
