You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by navis <gi...@git.apache.org> on 2015/03/06 06:25:11 UTC
[GitHub] tajo pull request: HIVE-1374 Support multi-bytes delimiter for CSV...
GitHub user navis opened a pull request:
https://github.com/apache/tajo/pull/400
HIVE-1374 Support multi-bytes delimiter for CSV file
Simple patch
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/navis/tajo HIVE-1374
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/400.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #400
----
commit ba795d4dd30e96350f6cc315b4feb21c51c619e2
Author: navis.ryu <na...@apache.org>
Date: 2015-03-06T05:19:15Z
HIVE-1374 Support multi-bytes delimiter for CSV file
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: HIVE-1374 Support multi-bytes delimiter for CSV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-77509534
Hi @navis,
Thank you for your contribution. There may be mistake in the title. Could you rename the title?
In these days, TravisCI has been too slow. There have been many pending pull requests for this reason. So, it would be better if you submit the patch to the jira and mark the jira as PATCH AVAILABLE. Please see this mailing thread too.
http://markmail.org/message/itfwyhcoppqmaouu
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: HIVE-1374 Support multi-bytes delimiter for CSV...
Posted by navis <gi...@git.apache.org>.
Github user navis commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-77793864
@jinossy If it will be not deprecated in near future, can we still get some usefulness in this? Fixed test fails, anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-77982695
@navis
OK, I've create new jira issue TAJO-1381.
If you fix test failure, we can start review.
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-77974324
The patch looks nice to me. Of course, your work is useful.
It would be great if this feature is applied to DelimitedTextFile too because DelimitedTextFile is new replacement to CSVFile. DelimitedTextFile's performance is really great. According to some benchmark result, it can parse more than 500MB CSV data sets per second. It also can boost up query response times in many cases, especially I/O intensive workloads.
I think that the work for DelimitedTextFile does not need to be done in this issue. We can do in another jira.
Anyway, could you fix some test failure? It still has one test failure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by navis <gi...@git.apache.org>.
Github user navis commented on a diff in the pull request:
https://github.com/apache/tajo/pull/400#discussion_r26098848
--- Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/LazyTuple.java ---
@@ -134,7 +134,7 @@ else if (textBytes.length <= fieldId) {
}
textBytes[fieldId] = null;
} else {
- //non-projection
+ values[fieldId] = NullDatum.get(); //non-projection
--- End diff --
reverted this part. I've hardly noticed tajo expects null for not-picked and byte[0] for not-existing picked.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by navis <gi...@git.apache.org>.
Github user navis commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-77995916
Fixed test fails
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/400
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: HIVE-1374 Support multi-bytes delimiter for CSV...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-77520488
@navis Good to see you
The CSVFile will be deprecated. please refer to DelimitedTextFile
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/400#issuecomment-78809964
+1 LGTM I'll commit it shortly.
Thank you for your contribution
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1374 Support multi-bytes delimiter for CSV...
Posted by ykrips <gi...@git.apache.org>.
Github user ykrips commented on a diff in the pull request:
https://github.com/apache/tajo/pull/400#discussion_r26091955
--- Diff: tajo-storage/tajo-storage-common/src/main/java/org/apache/tajo/storage/LazyTuple.java ---
@@ -134,7 +134,7 @@ else if (textBytes.length <= fieldId) {
}
textBytes[fieldId] = null;
} else {
- //non-projection
+ values[fieldId] = NullDatum.get(); //non-projection
--- End diff --
This code fills the NullDatum to values array of LazyTuple when user does not insert any data on this index. However, VTuple class does not initialize the values array on itself.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---