Return-Path: X-Original-To: apmail-sqoop-dev-archive@www.apache.org Delivered-To: apmail-sqoop-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1D295EB63 for ; Wed, 28 Nov 2012 20:26:00 +0000 (UTC) Received: (qmail 51710 invoked by uid 500); 28 Nov 2012 20:26:00 -0000 Delivered-To: apmail-sqoop-dev-archive@sqoop.apache.org Received: (qmail 51604 invoked by uid 500); 28 Nov 2012 20:25:59 -0000 Mailing-List: contact dev-help@sqoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sqoop.apache.org Delivered-To: mailing list dev@sqoop.apache.org Received: (qmail 51585 invoked by uid 500); 28 Nov 2012 20:25:59 -0000 Delivered-To: apmail-incubator-sqoop-dev@incubator.apache.org Received: (qmail 51570 invoked by uid 99); 28 Nov 2012 20:25:59 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 28 Nov 2012 20:25:59 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 0FDFE1C4DE8; Wed, 28 Nov 2012 20:25:54 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2754017895130944208==" MIME-Version: 1.0 Subject: Re: Review Request: SQOOP-724 Support Table hints in Microsoft SQL Server From: "Cheolsoo Park" To: "Sqoop" , "Jarek Cecho" , "Cheolsoo Park" Date: Wed, 28 Nov 2012 20:25:54 -0000 Message-ID: <20121128202554.22473.12784@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Cheolsoo Park" X-ReviewGroup: Sqoop X-ReviewRequest-URL: https://reviews.apache.org/r/8203/ X-Sender: "Cheolsoo Park" References: <20121124050723.3328.35767@reviews.apache.org> In-Reply-To: <20121124050723.3328.35767@reviews.apache.org> Reply-To: "Cheolsoo Park" --===============2754017895130944208== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8203/#review13830 ----------------------------------------------------------- Hi Jarcec, Overall looks good. I have a few minor comments inline. I will commit it on= ce you correct them, and all tests pass. Thanks! src/docs/user/connectors.txt shown on table below =3D> shown below src/docs/user/connectors.txt PostgresSQL =3D> MS SQL src/docs/user/connectors.txt Can you change this as follows? = If you need to work with tables that are located in non-default schemas= , you can specify schema names via the +\--schema+ argument. Custom schemas= are supported for both import and export jobs. For example: src/docs/user/connectors.txt Can you change this as follows? = Sqoop supports table hints in both import and export jobs. Table hints = are used only for queries that move data from/to Microsoft SQL Server, but = they cannot be used for meta data queries. You can specify a comma-separate= d list of table hints in the +\--table-hints+ argument. For example: src/docs/user/connectors.txt shown on table below =3D> shown below src/java/org/apache/sqoop/manager/SQLServerManager.java Can you put this line together with other com.cloudera.sqoop.* import s= tatements? = src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerExportBatchOutputFor= mat.java Please delete ".". src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerExportBatchOutputFor= mat.java Put a space before "{". src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerExportBatchOutputFor= mat.java Can you put an empty space after ")" ? src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerInputFormat.java DB2-specific =3D> MS SQL-specific src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java Can you please add another test case that passes a comma-separated list= of options? src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java Can you please add another test case that passes a comma-separated list= of options? - Cheolsoo Park On Nov. 24, 2012, 5:07 a.m., Jarek Cecho wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8203/ > ----------------------------------------------------------- > = > (Updated Nov. 24, 2012, 5:07 a.m.) > = > = > Review request for Sqoop. > = > = > Description > ------- > = > I've added support for table hints into Microsoft SQL Connector. > = > = > This addresses bug SQOOP-724. > https://issues.apache.org/jira/browse/SQOOP-724 > = > = > Diffs > ----- > = > src/docs/user/connectors.txt d912840a519d408d2f6272a0761dda1fdbcc8859 = > src/java/org/apache/sqoop/manager/SQLServerManager.java 51f867953c98509= 4bb0325f4a834d6b30d815c3c = > src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerExportBatchOutpu= tFormat.java PRE-CREATION = > src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerInputFormat.java= PRE-CREATION = > src/java/org/apache/sqoop/mapreduce/sqlserver/SqlServerRecordReader.jav= a PRE-CREATION = > src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.ja= va ac7a934a634e5bd89b4107d607db7327fc83c6b2 = > src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.ja= va bf889d00ca3d7c1d35acc3a6ffc7f0a2551b8b53 = > = > Diff: https://reviews.apache.org/r/8203/diff/ > = > = > Testing > ------- > = > I've added two unit tests that are more checking that new argument is wor= king then it's actual usage. It's not trivial to test "hints" when SQL Serv= er might decide not use them. I've also done real life testing on SQL Serve= r Express. > = > = > Thanks, > = > Jarek Cecho > = > --===============2754017895130944208==--