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

Predicate hashCode() should return different values among unequal objects for better performance #152

Closed
matthewadams opened this issue May 23, 2012 · 10 comments

Comments

@matthewadams
Copy link
Contributor

@matthewadams matthewadams commented May 23, 2012

It appears that BooleanExpression#hashCode() is implemented using fewer data than are considered in BooleanExpression#equals(Object). A test like the following demonstrates the issue. Replace QXxx with any Q-class that fits.

    @Test
    public void testQueryDslPredicateEqualsAndHashCode() {
        QXxx x = QXxx.xxx;

        Predicate p1 = x.foo.eq(1L);
        Predicate p2 = x.foo.eq(1L);

        assertEquals(p1.hashCode(), p2.hashCode());
        assertTrue(p1.equals(p2));

        Predicate p3 = x.foo.eq(2L);

        assertFalse(p1.equals(p3));
        assertFalse(p2.equals(p3));

        assertTrue("hashCodes should differ", p1.hashCode() != p3.hashCode());
    }

Since we're getting ready to use Predicate instances in a Map<Predicate,List>, I want to be sure that Predicate#equals(Object) & Predicate#hashCode() are implemented correctly.

The contract of Object#hashCode() states, in part: "...the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables." While it is technically possible for two unequal objects to produce the same hashcode, it is expected to be extremely unlikely.

@timowest
Copy link
Member

@timowest timowest commented May 23, 2012

I believe the contract is that if objects are equal then the hash codes need to be equal as well. But if objects are not equal, then hash codes can be equal or not.

http://docs.oracle.com/javase/1.4.2/docs/api/java/lang/Object.html#hashCode%28%29

@matthewadams
Copy link
Contributor Author

@matthewadams matthewadams commented May 23, 2012

True, but notice the last part of the third point describing the contract: "However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables." That's the part I was looking for. Performance is king, usually. :)

@timowest
Copy link
Member

@timowest timowest commented May 23, 2012

Ok, could you update the issue title, since it is misleading. Maybe "Improve hashCode implementation for Expressions", because that's what you are looking for.

@matthewadams
Copy link
Contributor Author

@matthewadams matthewadams commented May 23, 2012

Done. Thanks!

@timowest
Copy link
Member

@timowest timowest commented May 24, 2012

Improved hash code generation is now centrally defined in com.mysema.query.types.HashCodeVisitor

@timowest
Copy link
Member

@timowest timowest commented May 24, 2012

Some related refactorings were done to the Expression types, the types with the most changes are Coalesce from core and SumOver from the sql module

@matthewadams
Copy link
Contributor Author

@matthewadams matthewadams commented May 24, 2012

Thanks, Timo! In which release do you expect those changes to go into?

@timowest
Copy link
Member

@timowest timowest commented May 24, 2012

2.6.0. I will make the release end of May or beginning of June

@matthewadams
Copy link
Contributor Author

@matthewadams matthewadams commented May 24, 2012

Great, Timo. I really appreciate how responsive you are to your users' needs!

@timowest
Copy link
Member

@timowest timowest commented May 25, 2012

Released in 2.6.0

@timowest timowest closed this May 25, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants