sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Venkat Ranganathan <n....@live.com>
Subject Re: Review Request 44303: SQOOP-2863 Properly escape column names for generated INSERT statements
Date Fri, 04 Mar 2016 18:39:26 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44303/#review122125
-----------------------------------------------------------


Ship it!




> if they were depending on DB's default behavior of auto-lower/upper-casting, then this
will break and they will have to manually edit the --column argument to contain names as they
are persisted in the database catalog. I feel that this is reasonable thing to do as their
arguments are essentially wrong, but I'm wondering what others think?
+1 to this

- Venkat Ranganathan


On March 2, 2016, 3:25 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44303/
> -----------------------------------------------------------
> 
> (Updated March 2, 2016, 3:25 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2863
>     https://issues.apache.org/jira/browse/SQOOP-2863
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> The patch looks higher then necessary as I've decided to refactore the export tests to
properly escape column names to verify that everything works as it should - it wasn't strictly
necessary, but it's better outcome overall.
> 
> There is one thing that I want to point out explicitly - this change is not fully backward
compatible. For cases when users are using combination of export and --column argument - if
they were depending on DB's default behavior of auto-lower/upper-casting, then this will break
and they will have to manually edit the --column argument to contain names as they are persisted
in the database catalog. I feel that this is reasonable thing to do as their arguments are
essentially wrong, but I'm wondering what others think?
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/manager/ConnManager.java f98feb3 
>   src/java/org/apache/sqoop/mapreduce/JdbcCallExportJob.java 2459698 
>   src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 78df33c 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 8fa420e 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpsertExportJob.java 0a9bf7f 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 117cc3f

>   src/test/com/cloudera/sqoop/TestAvroExport.java 137a6e1 
>   src/test/com/cloudera/sqoop/TestExport.java 0b650af 
>   src/test/com/cloudera/sqoop/TestParquetExport.java 86b40fb 
>   src/test/com/cloudera/sqoop/testutil/ExportJobTestCase.java 9a6e8da 
>   src/test/org/apache/sqoop/TestExportUsingProcedure.java 98ebf3c 
> 
> Diff: https://reviews.apache.org/r/44303/diff/
> 
> 
> Testing
> -------
> 
> Unit tests passed and I've also verified few scenarios on real cluster against MySQL.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message