You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Dmitriy Ryaboy <dv...@gmail.com> on 2010/08/22 02:22:36 UTC
Caster interface and byte conversion
I just noticed that even though Utf8StorageConverter implements the various
byte[] toBytes(Obj o) methods, they are not part of the LoadCaster interface
-- and therefore can't be relied on when using modular Casters, like I am
trying to do for the HBaseLoader.
Since we don't want to introduce backwards-incompatible changes, I propose
adding a ByteCaster interface that defines these methods, and extending
Utf8StorageConverter to implement them (without actually changing the
implementation at all).
That way StoreFuncs that need to convert to bytes can use pluggable
converters. Objections?
-D
Re: Caster interface and byte conversion
Posted by Alan Gates <ga...@yahoo-inc.com>.
On Aug 24, 2010, at 1:22 PM, Dmitriy Ryaboy wrote:
> As far as the toBytes methods -- I am not sure what they were
> originally
> for. They aren't actually called anywhere that I can find, except my
> new
> HBase stuff.
> You are right, I could make it two interfaces, but I consolidated
> them for
> simplicity of use/implementation. Now that I think about it, I can
> put all
> the methods into StoreCaster and just have a unioning interface for
> simplicity:
>
> @InterfaceAudience.Public
> @InterfaceStability.Evolving
> public interface LoadStoreCaster extends LoadCaster, StoreCaster {
>
> }
>
> Does that seem ok?
Yeah, makes sense.
Alan.
>
> -D
>
> On Tue, Aug 24, 2010 at 10:01 AM, Alan Gates <ga...@yahoo-inc.com>
> wrote:
>
>> One other comment. By making this part of an interface that extends
>> LoadCaster you are assuming the implementing class is both a load
>> and store
>> function. It makes more sense to have a separate StoreCaster
>> interface
>> rather than extending LoadCaster.
>>
>> Alan.
>>
>>
>> On Aug 24, 2010, at 9:18 AM, Alan Gates wrote:
>>
>> This seems fine. Is the Pig engine at any point testing to see if
>> the
>>> interface is implemented and if so calling toBytes, or is this
>>> totally
>>> for use inside the store functions themselves to serialize Pig data
>>> types?
>>>
>>> Alan.
>>>
>>> On Aug 22, 2010, at 1:40 AM, Dmitriy Ryaboy wrote:
>>>
>>> The current HBase patch on PIG-1205 (patch 7) includes this
>>>> refactoring.
>>>> Please take a look if you have concerns.
>>>>
>>>> Or just if you feel like reviewing the code... :)
>>>>
>>>> -D
>>>>
>>>> On Sat, Aug 21, 2010 at 5:22 PM, Dmitriy Ryaboy
>>>> <dv...@gmail.com>
>>>> wrote:
>>>>
>>>> I just noticed that even though Utf8StorageConverter implements the
>>>>> various
>>>>> byte[] toBytes(Obj o) methods, they are not part of the LoadCaster
>>>>> interface
>>>>> -- and therefore can't be relied on when using modular Casters,
>>>>> like I
>>>>> am
>>>>> trying to do for the HBaseLoader.
>>>>>
>>>>> Since we don't want to introduce backwards-incompatible changes, I
>>>>> propose
>>>>> adding a ByteCaster interface that defines these methods, and
>>>>> extending
>>>>> Utf8StorageConverter to implement them (without actually
>>>>> changing the
>>>>> implementation at all).
>>>>> That way StoreFuncs that need to convert to bytes can use
>>>>> pluggable
>>>>> converters. Objections?
>>>>>
>>>>> -D
>>>>>
>>>>>
>>>
>>
Re: Caster interface and byte conversion
Posted by Dmitriy Ryaboy <dv...@gmail.com>.
As far as the toBytes methods -- I am not sure what they were originally
for. They aren't actually called anywhere that I can find, except my new
HBase stuff.
You are right, I could make it two interfaces, but I consolidated them for
simplicity of use/implementation. Now that I think about it, I can put all
the methods into StoreCaster and just have a unioning interface for
simplicity:
@InterfaceAudience.Public
@InterfaceStability.Evolving
public interface LoadStoreCaster extends LoadCaster, StoreCaster {
}
Does that seem ok?
-D
On Tue, Aug 24, 2010 at 10:01 AM, Alan Gates <ga...@yahoo-inc.com> wrote:
> One other comment. By making this part of an interface that extends
> LoadCaster you are assuming the implementing class is both a load and store
> function. It makes more sense to have a separate StoreCaster interface
> rather than extending LoadCaster.
>
> Alan.
>
>
> On Aug 24, 2010, at 9:18 AM, Alan Gates wrote:
>
> This seems fine. Is the Pig engine at any point testing to see if the
>> interface is implemented and if so calling toBytes, or is this totally
>> for use inside the store functions themselves to serialize Pig data
>> types?
>>
>> Alan.
>>
>> On Aug 22, 2010, at 1:40 AM, Dmitriy Ryaboy wrote:
>>
>> The current HBase patch on PIG-1205 (patch 7) includes this
>>> refactoring.
>>> Please take a look if you have concerns.
>>>
>>> Or just if you feel like reviewing the code... :)
>>>
>>> -D
>>>
>>> On Sat, Aug 21, 2010 at 5:22 PM, Dmitriy Ryaboy <dv...@gmail.com>
>>> wrote:
>>>
>>> I just noticed that even though Utf8StorageConverter implements the
>>>> various
>>>> byte[] toBytes(Obj o) methods, they are not part of the LoadCaster
>>>> interface
>>>> -- and therefore can't be relied on when using modular Casters, like I
>>>> am
>>>> trying to do for the HBaseLoader.
>>>>
>>>> Since we don't want to introduce backwards-incompatible changes, I
>>>> propose
>>>> adding a ByteCaster interface that defines these methods, and
>>>> extending
>>>> Utf8StorageConverter to implement them (without actually changing the
>>>> implementation at all).
>>>> That way StoreFuncs that need to convert to bytes can use pluggable
>>>> converters. Objections?
>>>>
>>>> -D
>>>>
>>>>
>>
>
Re: Caster interface and byte conversion
Posted by Alan Gates <ga...@yahoo-inc.com>.
One other comment. By making this part of an interface that extends
LoadCaster you are assuming the implementing class is both a load and
store function. It makes more sense to have a separate StoreCaster
interface rather than extending LoadCaster.
Alan.
On Aug 24, 2010, at 9:18 AM, Alan Gates wrote:
> This seems fine. Is the Pig engine at any point testing to see if the
> interface is implemented and if so calling toBytes, or is this totally
> for use inside the store functions themselves to serialize Pig data
> types?
>
> Alan.
>
> On Aug 22, 2010, at 1:40 AM, Dmitriy Ryaboy wrote:
>
>> The current HBase patch on PIG-1205 (patch 7) includes this
>> refactoring.
>> Please take a look if you have concerns.
>>
>> Or just if you feel like reviewing the code... :)
>>
>> -D
>>
>> On Sat, Aug 21, 2010 at 5:22 PM, Dmitriy Ryaboy <dv...@gmail.com>
>> wrote:
>>
>>> I just noticed that even though Utf8StorageConverter implements the
>>> various
>>> byte[] toBytes(Obj o) methods, they are not part of the LoadCaster
>>> interface
>>> -- and therefore can't be relied on when using modular Casters,
>>> like I am
>>> trying to do for the HBaseLoader.
>>>
>>> Since we don't want to introduce backwards-incompatible changes, I
>>> propose
>>> adding a ByteCaster interface that defines these methods, and
>>> extending
>>> Utf8StorageConverter to implement them (without actually changing
>>> the
>>> implementation at all).
>>> That way StoreFuncs that need to convert to bytes can use pluggable
>>> converters. Objections?
>>>
>>> -D
>>>
>
Re: Caster interface and byte conversion
Posted by Alan Gates <ga...@yahoo-inc.com>.
This seems fine. Is the Pig engine at any point testing to see if the
interface is implemented and if so calling toBytes, or is this totally
for use inside the store functions themselves to serialize Pig data
types?
Alan.
On Aug 22, 2010, at 1:40 AM, Dmitriy Ryaboy wrote:
> The current HBase patch on PIG-1205 (patch 7) includes this
> refactoring.
> Please take a look if you have concerns.
>
> Or just if you feel like reviewing the code... :)
>
> -D
>
> On Sat, Aug 21, 2010 at 5:22 PM, Dmitriy Ryaboy <dv...@gmail.com>
> wrote:
>
>> I just noticed that even though Utf8StorageConverter implements the
>> various
>> byte[] toBytes(Obj o) methods, they are not part of the LoadCaster
>> interface
>> -- and therefore can't be relied on when using modular Casters,
>> like I am
>> trying to do for the HBaseLoader.
>>
>> Since we don't want to introduce backwards-incompatible changes, I
>> propose
>> adding a ByteCaster interface that defines these methods, and
>> extending
>> Utf8StorageConverter to implement them (without actually changing the
>> implementation at all).
>> That way StoreFuncs that need to convert to bytes can use pluggable
>> converters. Objections?
>>
>> -D
>>
Re: Caster interface and byte conversion
Posted by Dmitriy Ryaboy <dv...@gmail.com>.
The current HBase patch on PIG-1205 (patch 7) includes this refactoring.
Please take a look if you have concerns.
Or just if you feel like reviewing the code... :)
-D
On Sat, Aug 21, 2010 at 5:22 PM, Dmitriy Ryaboy <dv...@gmail.com> wrote:
> I just noticed that even though Utf8StorageConverter implements the various
> byte[] toBytes(Obj o) methods, they are not part of the LoadCaster interface
> -- and therefore can't be relied on when using modular Casters, like I am
> trying to do for the HBaseLoader.
>
> Since we don't want to introduce backwards-incompatible changes, I propose
> adding a ByteCaster interface that defines these methods, and extending
> Utf8StorageConverter to implement them (without actually changing the
> implementation at all).
> That way StoreFuncs that need to convert to bytes can use pluggable
> converters. Objections?
>
> -D
>