You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Daniel Dai <da...@gmail.com> on 2011/03/09 23:11:55 UTC
Review Request: Fix piggybank unit test TestAllLoader, TestAvroStorage
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/485/
-----------------------------------------------------------
Review request for pig, Jakob Homan and Richard Ding.
Summary
-------
Two piggybank tests TestAllLoader, TestAvroStorage fail on trunk. We need to fix them.
TestAvroStorage is broken after PIG-1680. We now call LoadFunc.setLocation one more time. Need original author to take a look.
This addresses bug PIG-1890.
https://issues.apache.org/jira/browse/PIG-1890
Diffs
-----
http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/avro/AvroStorage.java 1079597
http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestAllLoader.java 1079597
Diff: https://reviews.apache.org/r/485/diff
Testing
-------
All piggybank tests pass. Since no change in Pig core code, ignore tests for pig core.
Thanks,
Daniel
Re: Review Request: Fix piggybank unit test TestAllLoader, TestAvroStorage
Posted by Lin Guo <gu...@gmail.com>.
Hi, Daniel,
In the test case, each input row is an array of floats.
We add the wrappers because of the following Pig requirements:
1. Each row has to be a tuple (that is why we introduce the outer PIG_WRAPPER);
2. Each element in a bag has to be a tuple (that is why we introduce
the inner PIG_WRAPPER).
We also want to remove the wrappers if possible.
Best,
Lin
On Mon, Mar 14, 2011 at 2:23 PM, Daniel Dai <ji...@yahoo-inc.com> wrote:
> This patch only address issue #1 mentioned in
> https://issues.apache.org/jira/browse/PIG-1890. There is another issue
> remain unaddressed:
>
> "2. The schema for AvroStorage seems to be wrong. For example, in first test
> case testArrayDefault, the schema for "in" is set to "PIG_WRAPPER: (FIELD:
> {PIG_WRAPPER: (ARRAY_ELEM: float)})". It seems PIG_WRAPPER is redundant.
> This issue is hidden until PIG-1188
> <https://issues.apache.org/jira/browse/PIG-1188> checked in."
>
> Lin and Jokob, will you take a look of that?
>
> Thanks
> Daniel
>
> On 03/13/2011 06:07 PM, Lin Guo wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/485/#review323
>> -----------------------------------------------------------
>>
>> Ship it!
>>
>>
>> - Lin
>>
>>
>> On 2011-03-09 14:11:54, Daniel Dai wrote:
>>>
>>> -----------------------------------------------------------
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/485/
>>> -----------------------------------------------------------
>>>
>>> (Updated 2011-03-09 14:11:54)
>>>
>>>
>>> Review request for pig, Jakob Homan and Richard Ding.
>>>
>>>
>>> Summary
>>> -------
>>>
>>> Two piggybank tests TestAllLoader, TestAvroStorage fail on trunk. We need
>>> to fix them.
>>>
>>> TestAvroStorage is broken after PIG-1680. We now call
>>> LoadFunc.setLocation one more time. Need original author to take a look.
>>>
>>>
>>> This addresses bug PIG-1890.
>>> https://issues.apache.org/jira/browse/PIG-1890
>>>
>>>
>>> Diffs
>>> -----
>>>
>>>
>>> http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/avro/AvroStorage.java
>>> 1079597
>>>
>>> http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestAllLoader.java
>>> 1079597
>>>
>>> Diff: https://reviews.apache.org/r/485/diff
>>>
>>>
>>> Testing
>>> -------
>>>
>>> All piggybank tests pass. Since no change in Pig core code, ignore tests
>>> for pig core.
>>>
>>>
>>> Thanks,
>>>
>>> Daniel
>>>
>>>
>
>
Re: Review Request: Fix piggybank unit test TestAllLoader, TestAvroStorage
Posted by Daniel Dai <ji...@yahoo-inc.com>.
This patch only address issue #1 mentioned in
https://issues.apache.org/jira/browse/PIG-1890. There is another issue
remain unaddressed:
"2. The schema for AvroStorage seems to be wrong. For example, in first
test case testArrayDefault, the schema for "in" is set to "PIG_WRAPPER:
(FIELD: {PIG_WRAPPER: (ARRAY_ELEM: float)})". It seems PIG_WRAPPER is
redundant. This issue is hidden until PIG-1188
<https://issues.apache.org/jira/browse/PIG-1188> checked in."
Lin and Jokob, will you take a look of that?
Thanks
Daniel
On 03/13/2011 06:07 PM, Lin Guo wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/485/#review323
> -----------------------------------------------------------
>
> Ship it!
>
>
> - Lin
>
>
> On 2011-03-09 14:11:54, Daniel Dai wrote:
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/485/
>> -----------------------------------------------------------
>>
>> (Updated 2011-03-09 14:11:54)
>>
>>
>> Review request for pig, Jakob Homan and Richard Ding.
>>
>>
>> Summary
>> -------
>>
>> Two piggybank tests TestAllLoader, TestAvroStorage fail on trunk. We need to fix them.
>>
>> TestAvroStorage is broken after PIG-1680. We now call LoadFunc.setLocation one more time. Need original author to take a look.
>>
>>
>> This addresses bug PIG-1890.
>> https://issues.apache.org/jira/browse/PIG-1890
>>
>>
>> Diffs
>> -----
>>
>> http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/avro/AvroStorage.java 1079597
>> http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestAllLoader.java 1079597
>>
>> Diff: https://reviews.apache.org/r/485/diff
>>
>>
>> Testing
>> -------
>>
>> All piggybank tests pass. Since no change in Pig core code, ignore tests for pig core.
>>
>>
>> Thanks,
>>
>> Daniel
>>
>>
Re: Review Request: Fix piggybank unit test TestAllLoader, TestAvroStorage
Posted by Lin Guo <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/485/#review323
-----------------------------------------------------------
Ship it!
- Lin
On 2011-03-09 14:11:54, Daniel Dai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/485/
> -----------------------------------------------------------
>
> (Updated 2011-03-09 14:11:54)
>
>
> Review request for pig, Jakob Homan and Richard Ding.
>
>
> Summary
> -------
>
> Two piggybank tests TestAllLoader, TestAvroStorage fail on trunk. We need to fix them.
>
> TestAvroStorage is broken after PIG-1680. We now call LoadFunc.setLocation one more time. Need original author to take a look.
>
>
> This addresses bug PIG-1890.
> https://issues.apache.org/jira/browse/PIG-1890
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/avro/AvroStorage.java 1079597
> http://svn.apache.org/repos/asf/pig/trunk/contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestAllLoader.java 1079597
>
> Diff: https://reviews.apache.org/r/485/diff
>
>
> Testing
> -------
>
> All piggybank tests pass. Since no change in Pig core code, ignore tests for pig core.
>
>
> Thanks,
>
> Daniel
>
>