Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-2637] [api-breaking] [scala, types] Adds equals and hashCode method to TypeInformations and TypeSerializers #1134

Closed
wants to merge 1 commit into from

Conversation

tillrohrmann
Copy link
Contributor

Adds abstract equals, hashCode, canEqual and toString methods to TypeInformation. Adds missing implementations to subtypes. The canEqual(Object obj) method returns true iff the obj can be equaled with the called instance.

Adds abstract equals, hashCode and canEqual methods to TypeSerializer.

Makes CompositeType subtypes serializable by removing non-serializable fields which were only used for the comparator construction. The comparator construction is now realized within a builder object which keeps the intermediate state. Consequently, the PR #943 is now obsolete and can be closed.

Refactors the ObjectArrayTypeInfo so that the type extraction logic now happens within the TypeExtractor and no longer in the TypeInformation subtype.

@rmetzger
Copy link
Contributor

Big change, but I didn't spot anything suspicious.
+1 to merge

/**
* Type information for numeric primitive types (int, long, double, byte, ...).
* Type information for numeric primitive types: int, long, double, byte, short, float, char,
* boolean.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Will remove it.

@fhueske
Copy link
Contributor

fhueske commented Sep 16, 2015

Very good refactoring! I had just a few inline comments.
Thanks for this effort!

@tillrohrmann
Copy link
Contributor Author

Thanks for reviewing @fhueske and @rmetzger. I addressed the comments and updated the PR.

@fhueske
Copy link
Contributor

fhueske commented Sep 16, 2015

Thanks for the fast update, LGTM.
Will merge this tomorrow unless somebody speaks up.

Long.class,
Byte.class,
Short.class,
Boolean.class,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Boolean numeric?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I missed that. Will remove it.

fhueske pushed a commit to fhueske/flink that referenced this pull request Sep 17, 2015
…o TypeInformations and TypeSerializers

- Fixes ObjectArrayTypeInfo
- Makes CompositeTypes serializable
- Adds test for equality relation's symmetric property
- Adds test for PojoTypeInfo serializability

This closes apache#1134
…ethod to TypeInformations and TypeSerializers. Fixes ObjectArrayTypeInfo. Makes CompositeTypes serializable.

Adds test for equality relation's symmetric property

Added test for PojoTypeInfo serializability
fhueske pushed a commit to fhueske/flink that referenced this pull request Sep 17, 2015
…o TypeInformations and TypeSerializers

- Fixes ObjectArrayTypeInfo
- Makes CompositeTypes serializable
- Adds test for equality relation's symmetric property
- Adds test for PojoTypeInfo serializability

This closes apache#1134
@asfgit asfgit closed this in 8ca853e Sep 17, 2015
@rxin
Copy link

rxin commented Sep 17, 2015

I understand you took the bot from Spark, but can you change the JIRA link? It is spamming the Spark JIRA right now.

https://issues.apache.org/jira/secure/ViewProfile.jspa?name=mxm

@rmetzger
Copy link
Contributor

Hey @rxin, I'm really sorry that this happened. Max told me that he stopped the application on AppEngine as soon as he learned that the tool is not working as expected.
As far as I can see the JIRA activity of Max's user account, the tool stopped changing Spark issues 3 hours ago.
Please let me know if more Spark JIRAs are affected.
I'll ask @mxm to remove the automated comments again and change the links back to their old value.

@mxm
Copy link
Contributor

mxm commented Sep 17, 2015

Hi @rxin, I'm really sorry for bothering. I was trying out the Spark Pull Request Dashboard with all the settings adapted to the Flink JIRA and Flink Github repository. Only when it was too late, I saw that there were some hard-coded URLs in the the source code that also needed to be adapted. This caused some unexpected behavior (Flink issue number in Flink pull requests being associated with Spark issues numbers). I've removed all comments and links and apologize very much for the inconvenience this caused. I've definitely learned a lesson out of this one.

Thanks for your patience and understanding!

@tillrohrmann tillrohrmann deleted the fixOptionTypeInfo branch September 24, 2015 15:20
nikste pushed a commit to nikste/flink that referenced this pull request Sep 29, 2015
…o TypeInformations and TypeSerializers

- Fixes ObjectArrayTypeInfo
- Makes CompositeTypes serializable
- Adds test for equality relation's symmetric property
- Adds test for PojoTypeInfo serializability

This closes apache#1134
lofifnc pushed a commit to lofifnc/flink that referenced this pull request Oct 8, 2015
…o TypeInformations and TypeSerializers

- Fixes ObjectArrayTypeInfo
- Makes CompositeTypes serializable
- Adds test for equality relation's symmetric property
- Adds test for PojoTypeInfo serializability

This closes apache#1134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants