Changing AR:CollectionAssociation#empty? to use #exists? #7270

Merged
merged 1 commit into from Aug 5, 2012

Conversation

Projects
None yet
4 participants
@beerlington
Contributor

beerlington commented Aug 5, 2012

COUNT(*) queries can be slow in PostgreSQL, #exists? avoids this by selecting a single record. @jonleighton suggested in issue #3179 that #empty? should be patched to use #exists? for the same benefit. I couldn't think of a great way to test it since the external behavior is not changing. If anyone has any suggestions for tests though, I would be more than happy to add them.

Changing AR:CollectionAssociation#empty? to use #exists?
COUNT(*) queries can be slow in PostgreSQL, #exists? avoids this by
selecting a single record.
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 5, 2012

Member

You can test asserting the generated SQL.

Member

rafaelfranca commented Aug 5, 2012

You can test asserting the generated SQL.

@jonleighton

This comment has been minimized.

Show comment
Hide comment
@jonleighton

jonleighton Aug 5, 2012

Member

The problem with asserting the SQL is that it's fragile. I think this is fine as it is, given it's just a perf optimisation.

Member

jonleighton commented Aug 5, 2012

The problem with asserting the SQL is that it's fragile. I think this is fine as it is, given it's just a perf optimisation.

jonleighton added a commit that referenced this pull request Aug 5, 2012

Merge pull request #7270 from beerlington/use_exists_for_empty
Changing AR:CollectionAssociation#empty? to use #exists?

@jonleighton jonleighton merged commit 6126f02 into rails:master Aug 5, 2012

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Sep 8, 2012

Member

@jonleighton since empty? was already patched to make use of exists?, I still think we could add present? (and maybe blank?) to the collection association handling as well, to avoid method_missing, wdyt? I can do the changes later if necessary. Thanks.

@jonleighton since empty? was already patched to make use of exists?, I still think we could add present? (and maybe blank?) to the collection association handling as well, to avoid method_missing, wdyt? I can do the changes later if necessary. Thanks.

@jonleighton

This comment has been minimized.

Show comment
Hide comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment