You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Paul Mazak <pm...@yahoo.com> on 2015/05/12 15:14:16 UTC
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/
-----------------------------------------------------------
(Updated May 12, 2015, 1:14 p.m.)
Review request for Sqoop and Yibing Shi.
Repository: sqoop-trunk
Description
-------
Supplied a patch that worked for us.
There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
Diffs
-----
src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
src/java/org/apache/sqoop/manager/SqlManager.java ead581d
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
Diff: https://reviews.apache.org/r/34104/diff/
Testing
-------
Integration testing passed exporting AVRO to MS SQL Server
Thanks,
Paul Mazak
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On Aug. 8, 2015, 6:47 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java, line 55
> > <https://reviews.apache.org/r/34104/diff/3-4/?file=1001751#file1001751line55>
> >
> > Let's again avoid the string comparison here?
For these, do you mind if I use assertEquals(a Set<Text>, the keySet()) ? Rather than do string compare or array compare.
For example:
assertEquals(asSetOfText("Age", "Name"), DefaultStringifier.load(job.getConfiguration(), AvroExportMapper.AVRO_COLUMN_TYPES_MAP, MapWritable.class).keySet());
private Set<Text> asSetOfText(String... strings) {
Set<Text> setOfText = new HashSet<>();
for (String string : strings)
setOfText.add(new Text(string));
return setOfText;
}
> On Aug. 8, 2015, 6:47 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/manager/SqlManagerTest.java, line 53
> > <https://reviews.apache.org/r/34104/diff/3-4/?file=1001750#file1001750line53>
> >
> > Nit: Can we also use the assertArrayEquals here? :)
Sorry - I previously thought assertArrayEquals wouldn't work in this case, but it does. I will fix this.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review94641
-----------------------------------------------------------
On Aug. 3, 2015, 6:41 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2015, 6:41 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On Aug. 8, 2015, 6:47 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java, line 55
> > <https://reviews.apache.org/r/34104/diff/3-4/?file=1001751#file1001751line55>
> >
> > Let's again avoid the string comparison here?
>
> Paul Mazak wrote:
> For these, do you mind if I use assertEquals(a Set<Text>, the keySet()) ? Rather than do string compare or array compare.
> For example:
> assertEquals(asSetOfText("Age", "Name"), DefaultStringifier.load(job.getConfiguration(), AvroExportMapper.AVRO_COLUMN_TYPES_MAP, MapWritable.class).keySet());
>
> private Set<Text> asSetOfText(String... strings) {
> Set<Text> setOfText = new HashSet<>();
> for (String string : strings)
> setOfText.add(new Text(string));
> return setOfText;
> }
>
> Jarek Cecho wrote:
> My goal is just to avoid string serialization and comparison.
>
> The suggestion seems reasonable, I think that Java will however let you write new HashSet<>() {"Age", "Name"} so you might not need the "asSetOfTest" method :)
>
> Paul Mazak wrote:
> It will, but I would have had to do new HashSet<>() {new Text("Age"), new Text("Name")} which seemed to take away from the readability of the test.
Technically, would have had to do: new HashSet<>(Arrays.asList(new Text("Age"), new Text("Name")));.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review94641
-----------------------------------------------------------
On Aug. 11, 2015, 7:36 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 7:36 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/SqoopOptions.java ace90fd
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/com/cloudera/sqoop/TestAvroExport.java 663828c
> src/test/org/apache/sqoop/TestSqoopOptions.java 87b340a
> src/test/org/apache/sqoop/manager/TestSqlManager.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On Aug. 8, 2015, 6:47 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java, line 55
> > <https://reviews.apache.org/r/34104/diff/3-4/?file=1001751#file1001751line55>
> >
> > Let's again avoid the string comparison here?
>
> Paul Mazak wrote:
> For these, do you mind if I use assertEquals(a Set<Text>, the keySet()) ? Rather than do string compare or array compare.
> For example:
> assertEquals(asSetOfText("Age", "Name"), DefaultStringifier.load(job.getConfiguration(), AvroExportMapper.AVRO_COLUMN_TYPES_MAP, MapWritable.class).keySet());
>
> private Set<Text> asSetOfText(String... strings) {
> Set<Text> setOfText = new HashSet<>();
> for (String string : strings)
> setOfText.add(new Text(string));
> return setOfText;
> }
>
> Jarek Cecho wrote:
> My goal is just to avoid string serialization and comparison.
>
> The suggestion seems reasonable, I think that Java will however let you write new HashSet<>() {"Age", "Name"} so you might not need the "asSetOfTest" method :)
It will, but I would have had to do new HashSet<>() {new Text("Age"), new Text("Name")} which seemed to take away from the readability of the test.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review94641
-----------------------------------------------------------
On Aug. 11, 2015, 7:36 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 7:36 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/SqoopOptions.java ace90fd
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/com/cloudera/sqoop/TestAvroExport.java 663828c
> src/test/org/apache/sqoop/TestSqoopOptions.java 87b340a
> src/test/org/apache/sqoop/manager/TestSqlManager.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Jarek Cecho <ja...@apache.org>.
> On Aug. 8, 2015, 6:47 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java, line 55
> > <https://reviews.apache.org/r/34104/diff/3-4/?file=1001751#file1001751line55>
> >
> > Let's again avoid the string comparison here?
>
> Paul Mazak wrote:
> For these, do you mind if I use assertEquals(a Set<Text>, the keySet()) ? Rather than do string compare or array compare.
> For example:
> assertEquals(asSetOfText("Age", "Name"), DefaultStringifier.load(job.getConfiguration(), AvroExportMapper.AVRO_COLUMN_TYPES_MAP, MapWritable.class).keySet());
>
> private Set<Text> asSetOfText(String... strings) {
> Set<Text> setOfText = new HashSet<>();
> for (String string : strings)
> setOfText.add(new Text(string));
> return setOfText;
> }
My goal is just to avoid string serialization and comparison.
The suggestion seems reasonable, I think that Java will however let you write new HashSet<>() {"Age", "Name"} so you might not need the "asSetOfTest" method :)
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review94641
-----------------------------------------------------------
On Aug. 11, 2015, 7:36 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 7:36 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/SqoopOptions.java ace90fd
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/com/cloudera/sqoop/TestAvroExport.java 663828c
> src/test/org/apache/sqoop/TestSqoopOptions.java 87b340a
> src/test/org/apache/sqoop/manager/TestSqlManager.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review94641
-----------------------------------------------------------
src/test/org/apache/sqoop/manager/SqlManagerTest.java (line 53)
<https://reviews.apache.org/r/34104/#comment149225>
Nit: Can we also use the assertArrayEquals here? :)
src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java (line 54)
<https://reviews.apache.org/r/34104/#comment149226>
Let's again avoid the string comparison here?
src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java (line 66)
<https://reviews.apache.org/r/34104/#comment149227>
Let's again avoid the string comparison here?
src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java (line 78)
<https://reviews.apache.org/r/34104/#comment149228>
Let's again avoid the string comparison here?
src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java (line 102)
<https://reviews.apache.org/r/34104/#comment149229>
Let's again avoid the string comparison here?
- Jarek Cecho
On Aug. 3, 2015, 6:41 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2015, 6:41 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review99034
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Aug. 11, 2015, 7:36 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 11, 2015, 7:36 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/SqoopOptions.java ace90fd
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/com/cloudera/sqoop/TestAvroExport.java 663828c
> src/test/org/apache/sqoop/TestSqoopOptions.java 87b340a
> src/test/org/apache/sqoop/manager/TestSqlManager.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/
-----------------------------------------------------------
(Updated Aug. 11, 2015, 7:36 p.m.)
Review request for Sqoop and Yibing Shi.
Changes
-------
Addressed code review comments. Added integration test to TestAvroExport. Fixed issue when SQL DB returns column names in a different case than the fields in Avro schema.
Repository: sqoop-trunk
Description
-------
Supplied a patch that worked for us.
There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
Diffs (updated)
-----
src/java/org/apache/sqoop/SqoopOptions.java ace90fd
src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
src/java/org/apache/sqoop/manager/SqlManager.java ead581d
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
src/test/com/cloudera/sqoop/TestAvroExport.java 663828c
src/test/org/apache/sqoop/TestSqoopOptions.java 87b340a
src/test/org/apache/sqoop/manager/TestSqlManager.java PRE-CREATION
src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java PRE-CREATION
Diff: https://reviews.apache.org/r/34104/diff/
Testing
-------
Integration testing passed exporting AVRO to MS SQL Server
Thanks,
Paul Mazak
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/
-----------------------------------------------------------
(Updated Aug. 3, 2015, 6:41 p.m.)
Review request for Sqoop and Yibing Shi.
Changes
-------
Addressed code review comments.
Repository: sqoop-trunk
Description
-------
Supplied a patch that worked for us.
There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
src/java/org/apache/sqoop/manager/SqlManager.java ead581d
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/34104/diff/
Testing
-------
Integration testing passed exporting AVRO to MS SQL Server
Thanks,
Paul Mazak
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Jarek Cecho <ja...@apache.org>.
> On July 22, 2015, 9:57 p.m., Jarek Cecho wrote:
> > I have few more comments:
> >
> > 1) The JIRA specifies that the user visible problem is that export from Avro won't work correctly. Can we add test for that, so that we know that the problem that we're trying to address is actually addressed? :)
>
> Paul Mazak wrote:
> Done. Before, I was testing the method Avro relies on - JdbcExportJob.configureGenericRecordExportInputFormat - implicitly. Now in the test, I set inputFileType to Avro and go through JdbcExportJob.configureInputFormat explicitly.
Thanks for adding the tests Paul. Can we also do end-to-end test that will verify that avro export with --column now works? Pretty much looking for a new test to this file:
https://github.com/apache/sqoop/blob/trunk/src/test/com/cloudera/sqoop/TestAvroExport.java
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review92659
-----------------------------------------------------------
On Aug. 3, 2015, 6:41 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2015, 6:41 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On July 22, 2015, 9:57 p.m., Jarek Cecho wrote:
> > I have few more comments:
> >
> > 1) The JIRA specifies that the user visible problem is that export from Avro won't work correctly. Can we add test for that, so that we know that the problem that we're trying to address is actually addressed? :)
Done. Before, I was testing the method Avro relies on - JdbcExportJob.configureGenericRecordExportInputFormat - implicitly. Now in the test, I set inputFileType to Avro and go through JdbcExportJob.configureInputFormat explicitly.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review92659
-----------------------------------------------------------
On July 7, 2015, 8:03 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated July 7, 2015, 8:03 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On July 22, 2015, 9:57 p.m., Jarek Cecho wrote:
> > src/test/org/apache/sqoop/manager/SqlManagerTest.java, line 41
> > <https://reviews.apache.org/r/34104/diff/3/?file=1001750#file1001750line41>
> >
> > Can we use AssertArrayEquals instead of the string serialization here?
Done.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review92659
-----------------------------------------------------------
On Aug. 3, 2015, 6:41 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2015, 6:41 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On July 22, 2015, 9:57 p.m., Jarek Cecho wrote:
> > I have few more comments:
> >
> > 1) The JIRA specifies that the user visible problem is that export from Avro won't work correctly. Can we add test for that, so that we know that the problem that we're trying to address is actually addressed? :)
>
> Paul Mazak wrote:
> Done. Before, I was testing the method Avro relies on - JdbcExportJob.configureGenericRecordExportInputFormat - implicitly. Now in the test, I set inputFileType to Avro and go through JdbcExportJob.configureInputFormat explicitly.
>
> Jarek Cecho wrote:
> Thanks for adding the tests Paul. Can we also do end-to-end test that will verify that avro export with --column now works? Pretty much looking for a new test to this file:
>
> https://github.com/apache/sqoop/blob/trunk/src/test/com/cloudera/sqoop/TestAvroExport.java
Yes, I will add a test to TestAvroExport. (I had trouble running it on Windows previously, but got it working now.)
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review92659
-----------------------------------------------------------
On Aug. 3, 2015, 6:41 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2015, 6:41 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review92659
-----------------------------------------------------------
I have few more comments:
1) The JIRA specifies that the user visible problem is that export from Avro won't work correctly. Can we add test for that, so that we know that the problem that we're trying to address is actually addressed? :)
src/test/org/apache/sqoop/manager/SqlManagerTest.java (line 41)
<https://reviews.apache.org/r/34104/#comment146886>
Can we use AssertArrayEquals instead of the string serialization here?
Jarcec
- Jarek Cecho
On July 7, 2015, 8:03 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated July 7, 2015, 8:03 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/
-----------------------------------------------------------
(Updated July 7, 2015, 8:03 p.m.)
Review request for Sqoop and Yibing Shi.
Changes
-------
Addressed code review comments to remove empty import line and check specifiedColumns == null || specifiedColumns.contains()
Added JdbcExportJobTest
Repository: sqoop-trunk
Description
-------
Supplied a patch that worked for us.
There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
src/java/org/apache/sqoop/manager/SqlManager.java ead581d
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/34104/diff/
Testing
-------
Integration testing passed exporting AVRO to MS SQL Server
Thanks,
Paul Mazak
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On July 7, 2015, 12:21 a.m., Abraham Elmahrek wrote:
> > src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java, line 104
> > <https://reviews.apache.org/r/34104/diff/2/?file=966944#file966944line104>
> >
> > Why not set this to "null" in some cases instead? Then the below condition could be:
> > if (specifiedColumns == null || specifiedColumns.contains())...
Fixed. Also made SqlManager.filterSpecifiedColumnNames not use .isEmpty() for consistency. Specified --columns option will have stuff in it or be null. Args parser does not allow empty.
Also, in order to test faster, added JdbcExportJobTest.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review90611
-----------------------------------------------------------
On July 7, 2015, 8:03 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated July 7, 2015, 8:03 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
> src/test/org/apache/sqoop/mapreduce/JdbcExportJobTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review90611
-----------------------------------------------------------
Ship it!
LGTM, just a couple of other things.
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java (line 26)
<https://reviews.apache.org/r/34104/#comment143724>
Unnecessary
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java (line 104)
<https://reviews.apache.org/r/34104/#comment143725>
Why not set this to "null" in some cases instead? Then the below condition could be:
if (specifiedColumns == null || specifiedColumns.contains())...
- Abraham Elmahrek
On May 21, 2015, 7:52 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 7:52 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
> src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/
-----------------------------------------------------------
(Updated May 21, 2015, 7:52 p.m.)
Review request for Sqoop and Yibing Shi.
Changes
-------
Addressed feedback
Repository: sqoop-trunk
Description
-------
Supplied a patch that worked for us.
There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
src/java/org/apache/sqoop/manager/OracleManager.java 69b613f
src/java/org/apache/sqoop/manager/SqlManager.java ead581d
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
src/test/org/apache/sqoop/manager/SqlManagerTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/34104/diff/
Testing
-------
Integration testing passed exporting AVRO to MS SQL Server
Thanks,
Paul Mazak
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On May 12, 2015, 8:14 p.m., Abraham Elmahrek wrote:
> > I think there are a few other places where getColumnNames is extended. Not sure if it's worth adding there.
> >
> > Also, I highly recommend adding tests to make sure this doesn't come back.
>
> Paul Mazak wrote:
> You're right. There are 4 places that Override public String[] getColumnNames(), and I had only addressed 3.
> CatalogQueryManager - Already addressed.
> MainframeManager - Not applicable. Returns literal 'DEFAULT_COLUMN'.
> #OracleManager - Good catch. Will address in next patch.
> SqlManager - Already addressed.
I'll add unit tests for SqlManager in upcoming patch.
SqlManagerTest
testFilteringSpecifiedColumnNamesWhenNoneSpecified()
testFilteringSpecifiedColumnNamesWhenSubset()
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review83442
-----------------------------------------------------------
On May 12, 2015, 1:14 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 1:14 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Paul Mazak <pm...@yahoo.com>.
> On May 12, 2015, 8:14 p.m., Abraham Elmahrek wrote:
> > I think there are a few other places where getColumnNames is extended. Not sure if it's worth adding there.
> >
> > Also, I highly recommend adding tests to make sure this doesn't come back.
You're right. There are 4 places that Override public String[] getColumnNames(), and I had only addressed 3.
CatalogQueryManager - Already addressed.
MainframeManager - Not applicable. Returns literal 'DEFAULT_COLUMN'.
#OracleManager - Good catch. Will address in next patch.
SqlManager - Already addressed.
> On May 12, 2015, 8:14 p.m., Abraham Elmahrek wrote:
> > src/java/org/apache/sqoop/manager/SqlManager.java, line 129
> > <https://reviews.apache.org/r/34104/diff/1/?file=956604#file956604line129>
> >
> > Can be a Set type like TreeSet. Some times users have hundreds to thousands of columns that they specify.
I like where you're going with performance so I changed to LinkedHashSet to preseve order from the --columns argument but also be faster when calling specifiedColumns.contains()
> On May 12, 2015, 8:14 p.m., Abraham Elmahrek wrote:
> > src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java, line 103
> > <https://reviews.apache.org/r/34104/diff/1/?file=956605#file956605line103>
> >
> > Use TreeSet?
I changed to LinkedHashSet in upcoming patch.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review83442
-----------------------------------------------------------
On May 12, 2015, 1:14 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 1:14 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>
Re: Review Request 34104: Fix for SQOOP-1369 Avro export ignores
--columns option
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34104/#review83442
-----------------------------------------------------------
I think there are a few other places where getColumnNames is extended. Not sure if it's worth adding there.
Also, I highly recommend adding tests to make sure this doesn't come back.
src/java/org/apache/sqoop/manager/SqlManager.java
<https://reviews.apache.org/r/34104/#comment134430>
Can be a Set type like TreeSet. Some times users have hundreds to thousands of columns that they specify.
src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java
<https://reviews.apache.org/r/34104/#comment134434>
Use TreeSet?
- Abraham Elmahrek
On May 12, 2015, 1:14 p.m., Paul Mazak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34104/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 1:14 p.m.)
>
>
> Review request for Sqoop and Yibing Shi.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Supplied a patch that worked for us.
> There are 2 global places to consider - parent class SqlManager and child class CatalogQueryManager - when returning getColumnNames to actually rely on those column names you specified with --columns option.
> There was 1 place to consider for Avro/Parquet in JdbcExportJob.configureGenericRecordExportInputFormat() to only add the columnType if it's one that was specified with --columns option.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/CatalogQueryManager.java 4e063ed
> src/java/org/apache/sqoop/manager/SqlManager.java ead581d
> src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 93d438a
>
> Diff: https://reviews.apache.org/r/34104/diff/
>
>
> Testing
> -------
>
> Integration testing passed exporting AVRO to MS SQL Server
>
>
> Thanks,
>
> Paul Mazak
>
>