Re: [DISCUSS] Drop "canEqual" from TypeInformation, TypeSerializer, etc.
I created https://issues.apache.org/jira/browse/FLINK-9798 > On 18. May 2018, at 16:10, Till Rohrmann 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 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 >>> >>> 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 (se...@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.
+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 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 > > >> 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 (se...@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.
+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 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 (se...@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.
+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 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 (se...@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.
+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 (se...@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
[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
