sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abraham Fine <...@abrahamfine.com>
Subject Re: Review Request 42327: SQOOP-2788: Parquet support for HdfsConnector
Date Fri, 22 Jan 2016 21:16:31 GMT


> On Jan. 21, 2016, 10:49 p.m., Ryan Blue wrote:
> > test/src/test/java/org/apache/sqoop/integration/connector/hdfs/ParquetTest.java,
line 55
> > <https://reviews.apache.org/r/42327/diff/8/?file=1204500#file1204500line55>
> >
> >     This only has positive test cases. I'd like to see at least a negative case
for null handling that I saw above. Any other cases where the CSV passed into the writer is
unexpected would be good to validate this isn't going to blow up at runtime when the data
producer does something unexpected.

it appears that we do not have tests for handling null values within our integration tests
and there is a bug with this functionality on the generic-jdbc-connector side of things. so
if you don't mind i will create another ticket to add integration tests for our connectors
to cover cases with null columns.


> On Jan. 21, 2016, 10:49 p.m., Ryan Blue wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopAvroUtils.java,
line 46
> > <https://reviews.apache.org/r/42327/diff/8/?file=1204496#file1204496line46>
> >
> >     I think this should be an error. Changing names without warning will confuse
users. If the name is invalid, then it should be mapped somehow or there should be an option
(defaulted to false) to turn on this behavior.

this should not actually be an issue because sqoop is inserting the quotes into the schema.
```
    String schemaName;
    if(fromJobConfig.fromJobConfig.tableName != null) {
      schemaName = executor.encloseIdentifiers(fromJobConfig.fromJobConfig.schemaName, fromJobConfig.fromJobConfig.tableName);
    } else {
      schemaName = "Query";
    }

    Schema schema = new Schema(schemaName);
```

In addition this schema is generated by us and namespaced by us:
```
public static final String SQOOP_SCHEMA_NAMESPACE = "org.apache.sqoop";
```

so i do not think that end user confusion is likely in this case.


- Abraham


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


On Jan. 22, 2016, 1:04 a.m., Abraham Fine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42327/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2016, 1:04 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2788
>     https://issues.apache.org/jira/browse/SQOOP-2788
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> read and write parquet in hdfsconnector
> 
> 
> Diffs
> -----
> 
>   connector/connector-hdfs/pom.xml 599631418ca63cc43d645c1ee1e7a73dc824b313 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java
a813c479a07d68e14ed49936f642e762e5b37437 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java
774221aaf5c8cdb8d26ca108fae239598b42229b 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartition.java
644de60581faf90ceb2fcef8d3e0544067791fcc 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToFormat.java
27d121f529ecb4d5bd79e2b8c74ab8f7cc15fb10 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/GenericHdfsWriter.java
2ccccc4a94a582c8b47ccdefa523d1fd1632e627 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/HdfsParquetWriter.java
PRE-CREATION 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/HdfsSequenceWriter.java
75c2e7ef192d7d9628e622cc3c5ef176e33a73d0 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/HdfsTextWriter.java
78cf9732fdb89689b04d43e4af70ca5a43732dbf 
>   connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java
11fcef2a38209c79928f582cf8aa03e889247f22 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopAvroUtils.java
985149cbb0d28b55a19d17076d996364d7f2ae90 
>   connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/AVROIntermediateDataFormat.java
d78fa8b72ecfe62eeec240e01597e7f2a7e4dd76 
>   pom.xml cb8a973abc96af1de905cebd80d30177cbaf1cb4 
>   test/pom.xml 644a9c7dbc746d4a3268532bdcf0babd4faaafba 
>   test/src/test/java/org/apache/sqoop/integration/connector/hdfs/ParquetTest.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/42327/diff/
> 
> 
> Testing
> -------
> 
> integration tests pass
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>


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