> On March 23, 2014, 7:37 a.m., Jihoon Son wrote:
> > Hi, JaeHwa.
> > This patch looks great to me. I left just one question below. 
> > 
> > Also, I have a suggestion about the test.
> > Although the null data generally occur in real applications, I think that 
> > our tests are not enough to test the null handling of Tajo.
> > Would you add a test to evaluate the null handling of SequenceFileAppender 
> > and SequenceFileScanner?

Thanks Jihoon.

It's good idea. I'll update my patch to evaluate the null handling of Tajo. :)


> On March 23, 2014, 7:37 a.m., Jihoon Son wrote:
> > tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java,
> >  line 421
> > <https://reviews.apache.org/r/19406/diff/1/?file=528051#file528051line421>
> >
> >     Do you have any reasons of using unmatched classes for input format 
> > class and output format class?

Because these are default configurations in hive.
If we create sequencefile format table, hive sets configurations as follows:
STORED AS INPUTFORMAT 
  'org.apache.hadoop.mapred.SequenceFileInputFormat' 
OUTPUTFORMAT 
  'org.apache.hadoop.hive.ql.io.HiveSequenceFileOutputFormat'


- Jung


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


On March 19, 2014, 3:44 p.m., Jung JaeHwa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19406/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 3:44 p.m.)
> 
> 
> Review request for Tajo.
> 
> 
> Bugs: TAJO-616
>     https://issues.apache.org/jira/browse/TAJO-616
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> Hadoop users can create a sequence file format, and hive users also can 
> create a table which was stored as SequenceFIle. So, we need to support 
> sequence file format as follows:
> 
> * Update parser and catalog for SequenceFile
> * Make serializer/deserializer configurable in SequenceFile
> * Scanner for SequenceFile
> * Support for compression/decompression of SequenceFile
> * Compatible to apache hive
> 
> 
> Diffs
> -----
> 
>   
> tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogConstants.java
>  fa23da1 
>   
> tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java
>  2a6727e 
>   tajo-catalog/tajo-catalog-common/src/main/proto/CatalogProtos.proto 35171cc 
>   
> tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java
>  5387673 
>   
> tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java
>  fc2fdda 
>   
> tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java
>  cb3973a 
>   tajo-common/src/main/java/org/apache/tajo/util/Bytes.java f9ba923 
>   
> tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/planner/physical/TestPhysicalPlanner.java
>  1975a57 
>   
> tajo-storage/src/main/java/org/apache/tajo/storage/sequencefile/SequenceFileAppender.java
>  PRE-CREATION 
>   
> tajo-storage/src/main/java/org/apache/tajo/storage/sequencefile/SequenceFileScanner.java
>  PRE-CREATION 
>   tajo-storage/src/main/resources/storage-default.xml 5bf4453 
>   
> tajo-storage/src/test/java/org/apache/tajo/storage/TestCompressionStorages.java
>  a776eb6 
>   tajo-storage/src/test/java/org/apache/tajo/storage/TestMergeScanner.java 
> f2a66d9 
>   tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java 
> 1b36b78 
>   tajo-storage/src/test/java/org/apache/tajo/storage/v2/TestStorages.java 
> 05be40a 
>   tajo-storage/src/test/resources/storage-default.xml 304af10 
> 
> Diff: https://reviews.apache.org/r/19406/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install -Phcatalog-0.12.0
> 
> 
> Thanks,
> 
> Jung JaeHwa
> 
>

Reply via email to