Opened 8 years ago

Closed 8 years ago

#5030 closed (fixed)

Remove COUNT(*) aggregate from when determining whether row exists

Reported by: zigiDev@… Owned by: adrian
Component: Database layer (models, ORM) Version: master
Severity: Keywords: model save
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Currently, the save method of the Model object uses an aggregate function to determine if a row with the specified primary key already exists. However, we don't need to know the count just whether the row exists or not. An alternative is to just simply SELECT 1 so if the row exists it will return 1, and if not we get a null. On PostgreSQL the COUNT(*) version takes over twice as long as the SELECT 1 version (it's not that significant but it is unnecessary work). Patch attached.

Attachments (1)

aggregateDiff.diff (957 bytes) - added by zigiDev@… 8 years ago.

Download all attachments as: .zip

Change History (10)

Changed 8 years ago by zigiDev@…


comment:1 Changed 8 years ago by adrian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Are you peeking over my shoulder or something? I *just* noticed this literally 5 minutes ago, when I was snooping around in Wow.

Yes, we should make this change.

comment:2 Changed 8 years ago by zigiDev@…

That is weird! :)

comment:3 Changed 8 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Accepted

Promoting to accepted because Adrian was obviously too shocked to do that himself. Just wanted to check, COUNT 1 works across all supported databases?

comment:4 Changed 8 years ago by adrian

The patch looks perfect, but I'm not in a position to test it against any database other than Postgres. Can somebody test this against all the other DBs?

comment:5 Changed 8 years ago by zigiDev@…

I've run the django test suite over both Sqlite and Postgres and all tests passed. However, I can't test MySQL, SQL Server or Oracle myself.

comment:6 Changed 8 years ago by scott@…

I ran the tests with a MySQL database against r5783. One test failed with and without the patch.

So the patch seems to work fine with MySQL.

comment:7 Changed 8 years ago by Simon G. <dev@…>

comment:8 Changed 8 years ago by Simon G. <dev@…>

  • Triage Stage changed from Accepted to Ready for checkin

Commenters to the above thread show that the SELECT 1 does work on Oracle and SQL Server and is more efficient. I've check the explain plans under SQlite and MySQL (both InnoDB and MyISAM tables) and SELECT 1 is more efficient on SQLite and exactly the same in MySQL.

comment:9 Changed 8 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

(In [5882]) Fixed #5030 -- Removed 'COUNT()' from when determining whether a row exists. We're now using a SELECT 1 LIMIT 1 instead, as it's more efficient in some databases. Thanks, zigiDev@… and the various folks who verified this patch works

Note: See TracTickets for help on using tickets.
Back to Top