You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Xuefu Zhang <xz...@cloudera.com> on 2013/06/02 16:03:15 UTC
Re: Review Request: Review Request for HIVE-4554 Failed to create a table
from existing file if file path has spaces
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11335/
-----------------------------------------------------------
(Updated June 2, 2013, 2:03 p.m.)
Review request for hive and Ashutosh Chauhan.
Changes
-------
Patch is updated based on review comments.
Description
-------
Patch includes fix and new test case.
This addresses bug HIVE-4554.
https://issues.apache.org/jira/browse/HIVE-4554
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java bd8d252
ql/src/test/queries/clientpositive/load_file_with_space_in_the_name.q PRE-CREATION
ql/src/test/results/clientpositive/load_file_with_space_in_the_name.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/11335/diff/
Testing
-------
Thanks,
Xuefu Zhang
Re: Review Request: Review Request for HIVE-4554 Failed to create a table
from existing file if file path has spaces
Posted by Ashutosh Chauhan <ha...@apache.org>.
> On June 3, 2013, 11:15 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java, line 273
> > <https://reviews.apache.org/r/11335/diff/3/?file=300140#file300140line273>
> >
> > Apart from this change, all other changes are contained within if(isLocal) block. Because of this it seems its possible it might be triggered for non-local paths as well. Can you test it for hdfs:// path which has spaces. If its easy, it will be good to add it in test, else manual test is fine as well.
>
> Xuefu Zhang wrote:
> I tried to add a testcase loading file at HDFS into a table without a success. Doing this requires an HDFS accessible from the test machine. Please let me know if you think there is mechanism. However, I did manually test the case, and it works fine for me. (It fails w/o the patch.)
Glad that its working. You can add this test-case for MinmrCliDriver . Just write a regular .q test file and then include that within minimr.query.files parameter in build-common.xml . Those testcases run against minicluster so you can access hdfs:// there.
- Ashutosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11335/#review21366
-----------------------------------------------------------
On June 3, 2013, 10:18 p.m., Xuefu Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11335/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:18 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Description
> -------
>
> Patch includes fix and new test case.
>
>
> This addresses bug HIVE-4554.
> https://issues.apache.org/jira/browse/HIVE-4554
>
>
> Diffs
> -----
>
> data/files/person PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java bd8d252
> ql/src/test/queries/clientpositive/load_file_with_space_in_the_name.q PRE-CREATION
> ql/src/test/results/clientpositive/load_file_with_space_in_the_name.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11335/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xuefu Zhang
>
>
Re: Review Request: Review Request for HIVE-4554 Failed to create a table
from existing file if file path has spaces
Posted by Xuefu Zhang <xz...@cloudera.com>.
> On June 3, 2013, 11:15 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java, line 273
> > <https://reviews.apache.org/r/11335/diff/3/?file=300140#file300140line273>
> >
> > Apart from this change, all other changes are contained within if(isLocal) block. Because of this it seems its possible it might be triggered for non-local paths as well. Can you test it for hdfs:// path which has spaces. If its easy, it will be good to add it in test, else manual test is fine as well.
I tried to add a testcase loading file at HDFS into a table without a success. Doing this requires an HDFS accessible from the test machine. Please let me know if you think there is mechanism. However, I did manually test the case, and it works fine for me. (It fails w/o the patch.)
- Xuefu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11335/#review21366
-----------------------------------------------------------
On June 3, 2013, 10:18 p.m., Xuefu Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11335/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:18 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Description
> -------
>
> Patch includes fix and new test case.
>
>
> This addresses bug HIVE-4554.
> https://issues.apache.org/jira/browse/HIVE-4554
>
>
> Diffs
> -----
>
> data/files/person PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java bd8d252
> ql/src/test/queries/clientpositive/load_file_with_space_in_the_name.q PRE-CREATION
> ql/src/test/results/clientpositive/load_file_with_space_in_the_name.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11335/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xuefu Zhang
>
>
Re: Review Request: Review Request for HIVE-4554 Failed to create a table
from existing file if file path has spaces
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11335/#review21366
-----------------------------------------------------------
Patch looks good, apart from one comment.
ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java
<https://reviews.apache.org/r/11335/#comment44301>
Apart from this change, all other changes are contained within if(isLocal) block. Because of this it seems its possible it might be triggered for non-local paths as well. Can you test it for hdfs:// path which has spaces. If its easy, it will be good to add it in test, else manual test is fine as well.
- Ashutosh Chauhan
On June 3, 2013, 10:18 p.m., Xuefu Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11335/
> -----------------------------------------------------------
>
> (Updated June 3, 2013, 10:18 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Description
> -------
>
> Patch includes fix and new test case.
>
>
> This addresses bug HIVE-4554.
> https://issues.apache.org/jira/browse/HIVE-4554
>
>
> Diffs
> -----
>
> data/files/person PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java bd8d252
> ql/src/test/queries/clientpositive/load_file_with_space_in_the_name.q PRE-CREATION
> ql/src/test/results/clientpositive/load_file_with_space_in_the_name.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11335/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Xuefu Zhang
>
>
Re: Review Request: Review Request for HIVE-4554 Failed to create a table
from existing file if file path has spaces
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11335/
-----------------------------------------------------------
(Updated June 4, 2013, 9:51 p.m.)
Review request for hive and Ashutosh Chauhan.
Changes
-------
Patch now includes testcase for HDFS files.
Description
-------
Patch includes fix and new test case.
This addresses bug HIVE-4554.
https://issues.apache.org/jira/browse/HIVE-4554
Diffs (updated)
-----
build-common.xml 43d8e9c
data/files/person PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java bd8d252
ql/src/test/queries/clientpositive/load_file_with_space_in_the_name.q PRE-CREATION
ql/src/test/queries/clientpositive/load_hdfs_file_with_space_in_the_name.q PRE-CREATION
ql/src/test/results/clientpositive/load_file_with_space_in_the_name.q.out PRE-CREATION
ql/src/test/results/clientpositive/load_hdfs_file_with_space_in_the_name.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/11335/diff/
Testing
-------
Thanks,
Xuefu Zhang
Re: Review Request: Review Request for HIVE-4554 Failed to create a table
from existing file if file path has spaces
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11335/
-----------------------------------------------------------
(Updated June 3, 2013, 10:18 p.m.)
Review request for hive and Ashutosh Chauhan.
Changes
-------
1. Added data input file to the new test case that was missing from previous patch.
2. Please note that review board doesn't show the added data file name correctly because of the space in it. However, applying the patch to the code base has no issue.
Description
-------
Patch includes fix and new test case.
This addresses bug HIVE-4554.
https://issues.apache.org/jira/browse/HIVE-4554
Diffs (updated)
-----
data/files/person PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java bd8d252
ql/src/test/queries/clientpositive/load_file_with_space_in_the_name.q PRE-CREATION
ql/src/test/results/clientpositive/load_file_with_space_in_the_name.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/11335/diff/
Testing
-------
Thanks,
Xuefu Zhang