You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by Jung JaeHwa <jh...@gruter.com> on 2014/02/10 16:39:36 UTC

Review Request 17901: TAJO-592: Improve HCatalogStore.

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

Review request for Tajo.


Bugs: TAJO-592
    https://issues.apache.org/jira/browse/TAJO-592


Repository: tajo


Description
-------

If hive table used default row format delimiter, hive doesn't return field.delim. As the result, tajo can't scan hive tables which used default row format delimiter. So, in this case, tajo must set field.delim to \001.
And current HCatalogStore just supports TextFile, it should supports RCFile.
At last, I found that HCatalogStore doesn't set compression and field.delim when it created tables. So,we should implement these.


Diffs
-----

  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 540ad0f 
  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java 2c2400a 
  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java 2a6e740 

Diff: https://reviews.apache.org/r/17901/diff/


Testing
-------

mvn clean install -Phcatalog-0.11.0
mvn clean install -Phcatalog-0.12.0


Thanks,

Jung JaeHwa


Re: Review Request 17901: TAJO-592: HCatalogStore should supports RCFile and default hive field delimiter.

Posted by Jihoon Son <ji...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17901/#review34186
-----------------------------------------------------------

Ship it!


+1. This patch looks good to me.
Please reflect the below comment before you commit.
Thanks!


tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java
<https://reviews.apache.org/r/17901/#comment64199>

    There is an erratum. It seems to be fixed to 'supported'.


- Jihoon Son


On Feb. 11, 2014, 11:02 p.m., Jung JaeHwa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17901/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 11:02 p.m.)
> 
> 
> Review request for Tajo.
> 
> 
> Bugs: TAJO-592
>     https://issues.apache.org/jira/browse/TAJO-592
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> If hive table used default row format delimiter, hive doesn't return field.delim. As the result, tajo can't scan hive tables which used default row format delimiter. So, in this case, tajo must set field.delim to \001.
> And current HCatalogStore just supports TextFile, it should supports RCFile.
> At last, I found that HCatalogStore doesn't set compression and field.delim when it created tables. So,we should implement these.
> 
> 
> Diffs
> -----
> 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 540ad0f 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java 2c2400a 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java 2a6e740 
> 
> Diff: https://reviews.apache.org/r/17901/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install -Phcatalog-0.11.0
> mvn clean install -Phcatalog-0.12.0
> 
> 
> Thanks,
> 
> Jung JaeHwa
> 
>


Re: Review Request 17901: TAJO-592: HCatalogStore should supports RCFile and default hive field delimiter.

Posted by Jung JaeHwa <jh...@gruter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17901/
-----------------------------------------------------------

(Updated Feb. 11, 2014, 2:02 p.m.)


Review request for Tajo.


Summary (updated)
-----------------

TAJO-592: HCatalogStore should supports RCFile and default hive field delimiter.


Bugs: TAJO-592
    https://issues.apache.org/jira/browse/TAJO-592


Repository: tajo


Description
-------

If hive table used default row format delimiter, hive doesn't return field.delim. As the result, tajo can't scan hive tables which used default row format delimiter. So, in this case, tajo must set field.delim to \001.
And current HCatalogStore just supports TextFile, it should supports RCFile.
At last, I found that HCatalogStore doesn't set compression and field.delim when it created tables. So,we should implement these.


Diffs
-----

  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 540ad0f 
  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java 2c2400a 
  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java 2a6e740 

Diff: https://reviews.apache.org/r/17901/diff/


Testing
-------

mvn clean install -Phcatalog-0.11.0
mvn clean install -Phcatalog-0.12.0


Thanks,

Jung JaeHwa


Re: Review Request 17901: TAJO-592: Improve HCatalogStore.

Posted by Jung JaeHwa <jh...@gruter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17901/
-----------------------------------------------------------

(Updated Feb. 11, 2014, 2 p.m.)


Review request for Tajo.


Changes
-------

I fixed some bugs.


Bugs: TAJO-592
    https://issues.apache.org/jira/browse/TAJO-592


Repository: tajo


Description
-------

If hive table used default row format delimiter, hive doesn't return field.delim. As the result, tajo can't scan hive tables which used default row format delimiter. So, in this case, tajo must set field.delim to \001.
And current HCatalogStore just supports TextFile, it should supports RCFile.
At last, I found that HCatalogStore doesn't set compression and field.delim when it created tables. So,we should implement these.


Diffs (updated)
-----

  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 540ad0f 
  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java 2c2400a 
  tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java 2a6e740 

Diff: https://reviews.apache.org/r/17901/diff/


Testing
-------

mvn clean install -Phcatalog-0.11.0
mvn clean install -Phcatalog-0.12.0


Thanks,

Jung JaeHwa


Re: Review Request 17901: TAJO-592: Improve HCatalogStore.

Posted by Hyunsik Choi <hy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17901/#review34075
-----------------------------------------------------------


Thank you for nice work. I leaved some trivial comments.


tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java
<https://reviews.apache.org/r/17901/#comment64050>

    'field.delim' and 'csvfile.delimiter' seem to be used frequently. I think that constant strings would be better than just literals.



tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java
<https://reviews.apache.org/r/17901/#comment64051>

    I think that RCFileOutputFormat.class.getSimpleName() would be better than the string literal "RCFileOutputFormat". You already use RCFileOutputFormat.class.getName() in HCatalogStore.java. It would be more consistent.


- Hyunsik Choi


On Feb. 11, 2014, 12:39 a.m., Jung JaeHwa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17901/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 12:39 a.m.)
> 
> 
> Review request for Tajo.
> 
> 
> Bugs: TAJO-592
>     https://issues.apache.org/jira/browse/TAJO-592
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> If hive table used default row format delimiter, hive doesn't return field.delim. As the result, tajo can't scan hive tables which used default row format delimiter. So, in this case, tajo must set field.delim to \001.
> And current HCatalogStore just supports TextFile, it should supports RCFile.
> At last, I found that HCatalogStore doesn't set compression and field.delim when it created tables. So,we should implement these.
> 
> 
> Diffs
> -----
> 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 540ad0f 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogUtil.java 2c2400a 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/test/java/org/apache/tajo/catalog/store/TestHCatalogStore.java 2a6e740 
> 
> Diff: https://reviews.apache.org/r/17901/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install -Phcatalog-0.11.0
> mvn clean install -Phcatalog-0.12.0
> 
> 
> Thanks,
> 
> Jung JaeHwa
> 
>