You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Stephan Ewen <se...@apache.org> on 2018/05/16 08:35:23 UTC

[DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.

Hi all!

As part of an attempt to simplify some code in the TypeInfo and
TypeSerializer area, I would like to drop the "canEqual" methods for the
following reason:

"canEqual()" is necessary to make proper equality checks across hierarchies
of types. This is for example useful in a collection API, stating for
example whether a List can be equal to a Collection if they have the same
contents. We don't have that here.

A certain type information (and serializer) is equal to another one if they
describe the same type, strictly. There is no necessity for cross hierarchy
checks.

This has also let to the situation that most type infos and serializers
implement just a dummy/default version of "canEqual". Many "equals()"
methods do not even call the other object's "canEqual", etc.

As a first step, we could simply deprecate the method and implement an
empty default, and remove all calls to that method.

Best,
Stephan

Re: [DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.

Posted by Aljoscha Krettek <al...@apache.org>.
I created https://issues.apache.org/jira/browse/FLINK-9798

> On 18. May 2018, at 16:10, Till Rohrmann <tr...@apache.org> wrote:
> 
> +1
> 
> If we don't have a hierarchy of serializers then this method has no right
> to exist.
> 
> Cheers,
> Till
> 
> On Wed, May 16, 2018 at 11:06 AM, Timo Walther <tw...@apache.org> wrote:
> 
>> +1
>> 
>> TypeInformation has too many methods that need to be implemented but
>> provide little benefit for Flink.
>> 
>> Am 16.05.18 um 10:55 schrieb Ted Yu:
>> 
>> +1 from me as well.
>>> 
>>> I checked a few serializer classes. The `equals` method on serializers
>>> contains the logic of `canEqual` method whose existence seems redundant.
>>> 
>>> On Wed, May 16, 2018 at 1:49 AM, Tzu-Li (Gordon) Tai <tzulitai@apache.org
>>>> 
>>> wrote:
>>> 
>>> +1.
>>>> 
>>>> Looking at the implementations of the `canEqual` method in several
>>>> serializers, it seems like all that is done is a check whether the object
>>>> is of the same serializer class.
>>>> We’ll have to be careful and double check all `equals` method on
>>>> serializers that may have relied on the `canEqual` method to perform the
>>>> preliminary type check.
>>>> Otherwise, this sounds good.
>>>> 
>>>> On 16 May 2018 at 4:35:47 PM, Stephan Ewen (sewen@apache.org) wrote:
>>>> 
>>>> Hi all!
>>>> 
>>>> As part of an attempt to simplify some code in the TypeInfo and
>>>> TypeSerializer area, I would like to drop the "canEqual" methods for the
>>>> following reason:
>>>> 
>>>> "canEqual()" is necessary to make proper equality checks across
>>>> hierarchies
>>>> of types. This is for example useful in a collection API, stating for
>>>> example whether a List can be equal to a Collection if they have the same
>>>> contents. We don't have that here.
>>>> 
>>>> A certain type information (and serializer) is equal to another one if
>>>> they
>>>> describe the same type, strictly. There is no necessity for cross
>>>> hierarchy
>>>> checks.
>>>> 
>>>> This has also let to the situation that most type infos and serializers
>>>> implement just a dummy/default version of "canEqual". Many "equals()"
>>>> methods do not even call the other object's "canEqual", etc.
>>>> 
>>>> As a first step, we could simply deprecate the method and implement an
>>>> empty default, and remove all calls to that method.
>>>> 
>>>> Best,
>>>> Stephan
>>>> 
>>>> 
>> 


Re: [DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.

Posted by Till Rohrmann <tr...@apache.org>.
+1

If we don't have a hierarchy of serializers then this method has no right
to exist.

Cheers,
Till

On Wed, May 16, 2018 at 11:06 AM, Timo Walther <tw...@apache.org> wrote:

> +1
>
> TypeInformation has too many methods that need to be implemented but
> provide little benefit for Flink.
>
> Am 16.05.18 um 10:55 schrieb Ted Yu:
>
> +1 from me as well.
>>
>> I checked a few serializer classes. The `equals` method on serializers
>> contains the logic of `canEqual` method whose existence seems redundant.
>>
>> On Wed, May 16, 2018 at 1:49 AM, Tzu-Li (Gordon) Tai <tzulitai@apache.org
>> >
>> wrote:
>>
>> +1.
>>>
>>> Looking at the implementations of the `canEqual` method in several
>>> serializers, it seems like all that is done is a check whether the object
>>> is of the same serializer class.
>>> We’ll have to be careful and double check all `equals` method on
>>> serializers that may have relied on the `canEqual` method to perform the
>>> preliminary type check.
>>> Otherwise, this sounds good.
>>>
>>> On 16 May 2018 at 4:35:47 PM, Stephan Ewen (sewen@apache.org) wrote:
>>>
>>> Hi all!
>>>
>>> As part of an attempt to simplify some code in the TypeInfo and
>>> TypeSerializer area, I would like to drop the "canEqual" methods for the
>>> following reason:
>>>
>>> "canEqual()" is necessary to make proper equality checks across
>>> hierarchies
>>> of types. This is for example useful in a collection API, stating for
>>> example whether a List can be equal to a Collection if they have the same
>>> contents. We don't have that here.
>>>
>>> A certain type information (and serializer) is equal to another one if
>>> they
>>> describe the same type, strictly. There is no necessity for cross
>>> hierarchy
>>> checks.
>>>
>>> This has also let to the situation that most type infos and serializers
>>> implement just a dummy/default version of "canEqual". Many "equals()"
>>> methods do not even call the other object's "canEqual", etc.
>>>
>>> As a first step, we could simply deprecate the method and implement an
>>> empty default, and remove all calls to that method.
>>>
>>> Best,
>>> Stephan
>>>
>>>
>

Re: [DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.

Posted by Timo Walther <tw...@apache.org>.
+1

TypeInformation has too many methods that need to be implemented but 
provide little benefit for Flink.

Am 16.05.18 um 10:55 schrieb Ted Yu:
> +1 from me as well.
>
> I checked a few serializer classes. The `equals` method on serializers
> contains the logic of `canEqual` method whose existence seems redundant.
>
> On Wed, May 16, 2018 at 1:49 AM, Tzu-Li (Gordon) Tai <tz...@apache.org>
> wrote:
>
>> +1.
>>
>> Looking at the implementations of the `canEqual` method in several
>> serializers, it seems like all that is done is a check whether the object
>> is of the same serializer class.
>> We’ll have to be careful and double check all `equals` method on
>> serializers that may have relied on the `canEqual` method to perform the
>> preliminary type check.
>> Otherwise, this sounds good.
>>
>> On 16 May 2018 at 4:35:47 PM, Stephan Ewen (sewen@apache.org) wrote:
>>
>> Hi all!
>>
>> As part of an attempt to simplify some code in the TypeInfo and
>> TypeSerializer area, I would like to drop the "canEqual" methods for the
>> following reason:
>>
>> "canEqual()" is necessary to make proper equality checks across
>> hierarchies
>> of types. This is for example useful in a collection API, stating for
>> example whether a List can be equal to a Collection if they have the same
>> contents. We don't have that here.
>>
>> A certain type information (and serializer) is equal to another one if
>> they
>> describe the same type, strictly. There is no necessity for cross
>> hierarchy
>> checks.
>>
>> This has also let to the situation that most type infos and serializers
>> implement just a dummy/default version of "canEqual". Many "equals()"
>> methods do not even call the other object's "canEqual", etc.
>>
>> As a first step, we could simply deprecate the method and implement an
>> empty default, and remove all calls to that method.
>>
>> Best,
>> Stephan
>>


Re: [DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.

Posted by Ted Yu <yu...@gmail.com>.
+1 from me as well.

I checked a few serializer classes. The `equals` method on serializers
contains the logic of `canEqual` method whose existence seems redundant.

On Wed, May 16, 2018 at 1:49 AM, Tzu-Li (Gordon) Tai <tz...@apache.org>
wrote:

> +1.
>
> Looking at the implementations of the `canEqual` method in several
> serializers, it seems like all that is done is a check whether the object
> is of the same serializer class.
> We’ll have to be careful and double check all `equals` method on
> serializers that may have relied on the `canEqual` method to perform the
> preliminary type check.
> Otherwise, this sounds good.
>
> On 16 May 2018 at 4:35:47 PM, Stephan Ewen (sewen@apache.org) wrote:
>
> Hi all!
>
> As part of an attempt to simplify some code in the TypeInfo and
> TypeSerializer area, I would like to drop the "canEqual" methods for the
> following reason:
>
> "canEqual()" is necessary to make proper equality checks across
> hierarchies
> of types. This is for example useful in a collection API, stating for
> example whether a List can be equal to a Collection if they have the same
> contents. We don't have that here.
>
> A certain type information (and serializer) is equal to another one if
> they
> describe the same type, strictly. There is no necessity for cross
> hierarchy
> checks.
>
> This has also let to the situation that most type infos and serializers
> implement just a dummy/default version of "canEqual". Many "equals()"
> methods do not even call the other object's "canEqual", etc.
>
> As a first step, we could simply deprecate the method and implement an
> empty default, and remove all calls to that method.
>
> Best,
> Stephan
>

Re: [DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.

Posted by "Tzu-Li (Gordon) Tai" <tz...@apache.org>.
+1.

Looking at the implementations of the `canEqual` method in several serializers, it seems like all that is done is a check whether the object is of the same serializer class.
We’ll have to be careful and double check all `equals` method on serializers that may have relied on the `canEqual` method to perform the preliminary type check.
Otherwise, this sounds good.

On 16 May 2018 at 4:35:47 PM, Stephan Ewen (sewen@apache.org) wrote:

Hi all!  

As part of an attempt to simplify some code in the TypeInfo and  
TypeSerializer area, I would like to drop the "canEqual" methods for the  
following reason:  

"canEqual()" is necessary to make proper equality checks across hierarchies  
of types. This is for example useful in a collection API, stating for  
example whether a List can be equal to a Collection if they have the same  
contents. We don't have that here.  

A certain type information (and serializer) is equal to another one if they  
describe the same type, strictly. There is no necessity for cross hierarchy  
checks.  

This has also let to the situation that most type infos and serializers  
implement just a dummy/default version of "canEqual". Many "equals()"  
methods do not even call the other object's "canEqual", etc.  

As a first step, we could simply deprecate the method and implement an  
empty default, and remove all calls to that method.  

Best,  
Stephan