You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Mengwei Ding <me...@gmail.com> on 2013/07/10 21:02:21 UTC
Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if partition
column value is NULL
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/
-----------------------------------------------------------
Review request for Sqoop and Jarek Cecho.
Bugs: SQOOP-1049
https://issues.apache.org/jira/browse/SQOOP-1049
Repository: sqoop-sqoop2
Description
-------
commit 47e73c30b49be0168459d76bf8993205c7a4f4fc
Author: Mengwei Ding <me...@gmail.com>
Date: Wed Jul 10 11:41:05 2013 -0700
SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
:100644 100644 abcc89d... a940d15... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
:100644 100644 671bb4a... d331ae8... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java
:100644 100644 96818ba... 357fefb... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
:100644 100644 4401800... ff80ed3... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
Diffs
-----
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java 671bb4a
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 4401800
Diff: https://reviews.apache.org/r/12451/diff/
Testing
-------
Have done a manual test, in which I successfully import a table with some null values in partition column.
Thanks,
Mengwei Ding
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
> On July 17, 2013, 9:09 p.m., Jarek Cecho wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java, line 33
> > <https://reviews.apache.org/r/12451/diff/3/?file=321128#file321128line33>
> >
> > Nit: I would suggest to simplify the name a bit. Like partitionColumnNull or something like that.
Thank you, Jarcec.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23333
-----------------------------------------------------------
On July 12, 2013, 11:12 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 12, 2013, 11:12 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40709543f87c4a6f8fc6e8d7ad124cb4e917185d
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... eb9f3d2... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 96818ba... 86a3f8a... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 7792c57... 8f006ab... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
> :100644 100644 ef27236... e114df4... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
> :100644 100644 44fc984... 0f45090... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
> :100644 100644 7ecc900... 30ae4f0... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
> connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java 7ecc900
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
> On July 17, 2013, 9:09 p.m., Jarek Cecho wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java, lines 205-207
> > <https://reviews.apache.org/r/12451/diff/3/?file=321126#file321126line205>
> >
> > The deserialization to context object do not seem to be necessary. The configuration objects are available on all steps in the job workflow.
Yes, thank you, Jarcec.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23333
-----------------------------------------------------------
On July 12, 2013, 11:12 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 12, 2013, 11:12 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40709543f87c4a6f8fc6e8d7ad124cb4e917185d
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... eb9f3d2... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 96818ba... 86a3f8a... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 7792c57... 8f006ab... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
> :100644 100644 ef27236... e114df4... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
> :100644 100644 44fc984... 0f45090... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
> :100644 100644 7ecc900... 30ae4f0... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
> connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java 7ecc900
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23333
-----------------------------------------------------------
Hi Mengwei, thank you very much for incorporating my suggestions!
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
<https://reviews.apache.org/r/12451/#comment47214>
The deserialization to context object do not seem to be necessary. The configuration objects are available on all steps in the job workflow.
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
<https://reviews.apache.org/r/12451/#comment47217>
Nit: I would suggest to simplify the name a bit. Like partitionColumnNull or something like that.
connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
<https://reviews.apache.org/r/12451/#comment47215>
Nit: The hint [true/false] is not necessary. The input itself do have boolean type.
Suggested rewording:
Nulls in partition column:
connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
<https://reviews.apache.org/r/12451/#comment47216>
Nit: s/be/contain/
Jarcec
- Jarek Cecho
On July 12, 2013, 11:12 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 12, 2013, 11:12 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 40709543f87c4a6f8fc6e8d7ad124cb4e917185d
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... eb9f3d2... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 96818ba... 86a3f8a... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 7792c57... 8f006ab... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
> :100644 100644 ef27236... e114df4... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
> :100644 100644 44fc984... 0f45090... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
> :100644 100644 7ecc900... 30ae4f0... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
> connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java 7ecc900
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23598
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On July 18, 2013, 8:50 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 18, 2013, 8:50 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit c41d3e475700c4d2f97606b153d75a86ef543cf4
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 f3dee8e... 27db8af... M common/src/main/java/org/apache/sqoop/model/FormUtils.java
> :100644 100644 7792c57... 8d0c4ab... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
> :100644 100644 ef27236... 0991b28... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
> :100644 100644 44fc984... 0950e32... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
> :100644 100644 7ecc900... a33dd6c... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/FormUtils.java f3dee8e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
> connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java 7ecc900
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/
-----------------------------------------------------------
(Updated July 18, 2013, 8:50 p.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
remove PartitionNull option in the context, because it could directly be fetch via JobConfig.
Bugs: SQOOP-1049
https://issues.apache.org/jira/browse/SQOOP-1049
Repository: sqoop-sqoop2
Description (updated)
-------
commit c41d3e475700c4d2f97606b153d75a86ef543cf4
Author: Mengwei Ding <me...@gmail.com>
Date: Wed Jul 10 11:41:05 2013 -0700
SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
:100644 100644 f3dee8e... 27db8af... M common/src/main/java/org/apache/sqoop/model/FormUtils.java
:100644 100644 7792c57... 8d0c4ab... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
:100644 100644 ef27236... 0991b28... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
:100644 100644 44fc984... 0950e32... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
:100644 100644 7ecc900... a33dd6c... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/FormUtils.java f3dee8e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java 7ecc900
Diff: https://reviews.apache.org/r/12451/diff/
Testing
-------
Have done a manual test, in which I successfully import a table with some null values in partition column.
Thanks,
Mengwei Ding
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/
-----------------------------------------------------------
(Updated July 12, 2013, 11:12 p.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
Add test case.
Bugs: SQOOP-1049
https://issues.apache.org/jira/browse/SQOOP-1049
Repository: sqoop-sqoop2
Description (updated)
-------
commit 40709543f87c4a6f8fc6e8d7ad124cb4e917185d
Author: Mengwei Ding <me...@gmail.com>
Date: Wed Jul 10 11:41:05 2013 -0700
SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
:100644 100644 abcc89d... eb9f3d2... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
:100644 100644 96818ba... 86a3f8a... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
:100644 100644 7792c57... 8f006ab... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
:100644 100644 ef27236... e114df4... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
:100644 100644 44fc984... 0f45090... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
:100644 100644 7ecc900... 30ae4f0... M connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java
Diffs (updated)
-----
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestImportPartitioner.java 7ecc900
Diff: https://reviews.apache.org/r/12451/diff/
Testing
-------
Have done a manual test, in which I successfully import a table with some null values in partition column.
Thanks,
Mengwei Ding
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/
-----------------------------------------------------------
(Updated July 12, 2013, 10:08 p.m.)
Review request for Sqoop and Jarek Cecho.
Changes
-------
Now, let the user input whether there could be nulls in the given partition column.
Bugs: SQOOP-1049
https://issues.apache.org/jira/browse/SQOOP-1049
Repository: sqoop-sqoop2
Description (updated)
-------
commit b9b99bd826f1386b84525084b9b8145d40b8768f
Author: Mengwei Ding <me...@gmail.com>
Date: Wed Jul 10 11:41:05 2013 -0700
SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
:100644 100644 abcc89d... eb9f3d2... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
:100644 100644 96818ba... e268b08... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
:100644 100644 7792c57... a907ca7... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
:100644 100644 ef27236... e114df4... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java
:100644 100644 44fc984... 0f45090... M connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties
Diffs (updated)
-----
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 7792c57
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ImportTableForm.java ef27236
connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-resources.properties 44fc984
Diff: https://reviews.apache.org/r/12451/diff/
Testing
-------
Have done a manual test, in which I successfully import a table with some null values in partition column.
Thanks,
Mengwei Ding
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
> On July 11, 2013, 9:54 p.m., Jarek Cecho wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java, line 184
> > <https://reviews.apache.org/r/12451/diff/1/?file=319957#file319957line184>
> >
> > I'm concerned a bit of using count() aggregate function as it might lead to another full table scan which might significantly hurt performance. Maybe we could make the ability for checking nulls in the split by column optional?
>
> Mengwei Ding wrote:
> Yes, this is an issue. I will use 'count(1)' instead.
>
> Jarek Cecho wrote:
> I'm afraid that count(1) won't help either. In case that the database engine is not storing the precise number of columns (such as InnoDB in MySQL), queries of type "select count(*/1) from table" will result in full table scan, which might be quite heavy operation.
>
> Mengwei Ding wrote:
> Yes, I did some research just now. For null values, they won't be indexed in database. Thus, to retrieve all null values, it has to scan the whole table. I just thought out another idea that we don't necessarily need to check whether the column has nulls, instead we could add an extra partition for nulls at any time. In this way, we reduce the full table scan to one, since we cannot avoid full table scan. By the way, what do you mean by checking nulls in the split by column optional ?
Will ask user whether the partition column could be null.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23028
-----------------------------------------------------------
On July 10, 2013, 7:02 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 7:02 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 47e73c30b49be0168459d76bf8993205c7a4f4fc
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... a940d15... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 671bb4a... d331ae8... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java
> :100644 100644 96818ba... 357fefb... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 4401800... ff80ed3... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java 671bb4a
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 4401800
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Jarek Cecho <ja...@apache.org>.
> On July 11, 2013, 9:54 p.m., Jarek Cecho wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java, line 184
> > <https://reviews.apache.org/r/12451/diff/1/?file=319957#file319957line184>
> >
> > I'm concerned a bit of using count() aggregate function as it might lead to another full table scan which might significantly hurt performance. Maybe we could make the ability for checking nulls in the split by column optional?
>
> Mengwei Ding wrote:
> Yes, this is an issue. I will use 'count(1)' instead.
I'm afraid that count(1) won't help either. In case that the database engine is not storing the precise number of columns (such as InnoDB in MySQL), queries of type "select count(*/1) from table" will result in full table scan, which might be quite heavy operation.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23028
-----------------------------------------------------------
On July 10, 2013, 7:02 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 7:02 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 47e73c30b49be0168459d76bf8993205c7a4f4fc
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... a940d15... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 671bb4a... d331ae8... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java
> :100644 100644 96818ba... 357fefb... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 4401800... ff80ed3... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java 671bb4a
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 4401800
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
> On July 11, 2013, 9:54 p.m., Jarek Cecho wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java, line 184
> > <https://reviews.apache.org/r/12451/diff/1/?file=319957#file319957line184>
> >
> > I'm concerned a bit of using count() aggregate function as it might lead to another full table scan which might significantly hurt performance. Maybe we could make the ability for checking nulls in the split by column optional?
>
> Mengwei Ding wrote:
> Yes, this is an issue. I will use 'count(1)' instead.
>
> Jarek Cecho wrote:
> I'm afraid that count(1) won't help either. In case that the database engine is not storing the precise number of columns (such as InnoDB in MySQL), queries of type "select count(*/1) from table" will result in full table scan, which might be quite heavy operation.
Yes, I did some research just now. For null values, they won't be indexed in database. Thus, to retrieve all null values, it has to scan the whole table. I just thought out another idea that we don't necessarily need to check whether the column has nulls, instead we could add an extra partition for nulls at any time. In this way, we reduce the full table scan to one, since we cannot avoid full table scan. By the way, what do you mean by checking nulls in the split by column optional ?
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23028
-----------------------------------------------------------
On July 10, 2013, 7:02 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 7:02 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 47e73c30b49be0168459d76bf8993205c7a4f4fc
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... a940d15... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 671bb4a... d331ae8... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java
> :100644 100644 96818ba... 357fefb... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 4401800... ff80ed3... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java 671bb4a
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 4401800
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Mengwei Ding <me...@gmail.com>.
> On July 11, 2013, 9:54 p.m., Jarek Cecho wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java, line 184
> > <https://reviews.apache.org/r/12451/diff/1/?file=319957#file319957line184>
> >
> > I'm concerned a bit of using count() aggregate function as it might lead to another full table scan which might significantly hurt performance. Maybe we could make the ability for checking nulls in the split by column optional?
Yes, this is an issue. I will use 'count(1)' instead.
- Mengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23028
-----------------------------------------------------------
On July 10, 2013, 7:02 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 7:02 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 47e73c30b49be0168459d76bf8993205c7a4f4fc
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... a940d15... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 671bb4a... d331ae8... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java
> :100644 100644 96818ba... 357fefb... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 4401800... ff80ed3... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java 671bb4a
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 4401800
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>
Re: Review Request 12451: SQOOP-1049: Sqoop2: Record not imported if
partition column value is NULL
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12451/#review23028
-----------------------------------------------------------
Hi Mengwei,
thank you very much for identifying this issue and providing the initial patch. Would you mind adding test cases testing the new functionality?
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
<https://reviews.apache.org/r/12451/#comment46841>
I'm concerned a bit of using count() aggregate function as it might lead to another full table scan which might significantly hurt performance. Maybe we could make the ability for checking nulls in the split by column optional?
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
<https://reviews.apache.org/r/12451/#comment46840>
Nit: This check seems to be redundant as the query is always hardcoded and is selecting one single value.
Jarcec
- Jarek Cecho
On July 10, 2013, 7:02 p.m., Mengwei Ding wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12451/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 7:02 p.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Bugs: SQOOP-1049
> https://issues.apache.org/jira/browse/SQOOP-1049
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> commit 47e73c30b49be0168459d76bf8993205c7a4f4fc
> Author: Mengwei Ding <me...@gmail.com>
> Date: Wed Jul 10 11:41:05 2013 -0700
>
> SQOOP-1049: Sqoop2: Record not imported if partition column value is NULL
>
> :100644 100644 abcc89d... a940d15... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java
> :100644 100644 671bb4a... d331ae8... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java
> :100644 100644 96818ba... 357fefb... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java
> :100644 100644 4401800... ff80ed3... M connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorConstants.java abcc89d
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java 671bb4a
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportInitializer.java 96818ba
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcImportPartitioner.java 4401800
>
> Diff: https://reviews.apache.org/r/12451/diff/
>
>
> Testing
> -------
>
> Have done a manual test, in which I successfully import a table with some null values in partition column.
>
>
> Thanks,
>
> Mengwei Ding
>
>