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

[OPENJPA-2788] addressing issue with anonymous parameters #42

Closed

Conversation

michaelwiles
Copy link

So a slight adjustment to the hashcode and equals method of the ParameterExpression.

Not 100% sure it's right like this but it works.

I guess I'm not too familiar with the reason for the addition of the hashCode and equals in the first place.

The bottom line is that we must include in these hashCode and equals method the ability to handle the situation where the name is not specified.

Copy link
Member

@ilgrosso ilgrosso left a comment

Choose a reason for hiding this comment

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

@michaelwiles your changes look good to me, and build goes fine.

@struberg / @rmannibucau any objection to go ahead and merge?

@rmannibucau
Copy link
Contributor

Looks rational to me but requires more testing. Typical case I have in mind is duplicate parameter names. Equals impl was intended for container (lists/sets) and paramEquals to handle these cases.
So long story short if we get a complete coverage of ambiguous parameters I'm fine with this change.

@ilgrosso
Copy link
Member

@michaelwiles can you address @rmannibucau 's comment? Thanks!

@michaelwiles
Copy link
Author

michaelwiles commented Oct 22, 2019 via email

@rmannibucau
Copy link
Contributor

From memory twice the same name with same type or not and same value or not - no more sure if both cases were needed, @struberg: do you recall?

@struberg
Copy link
Member

Hi! sorry for taking so long! I opted for a slightly less impacting change. Anynomous params are now only treated as the same if they are the same instance.
The whole case is imo rather underspecified in the JPA spec. But I think it's now as good as it gets.

@struberg struberg closed this Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants