Opened 17 years ago

Closed 17 years ago

#5030 closed (fixed)

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

Reported by: zigiDev@… Owned by: Adrian Holovaty
Component: Database layer (models, ORM) Version: dev
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: no UI/UX: no

Description

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@… 17 years ago.
Patch

Download all attachments as: .zip

Change History (10)

by zigiDev@…, 17 years ago

Attachment: aggregateDiff.diff​ added

Patch

comment:1 by Adrian Holovaty, 17 years ago

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

Yes, we should make this change.

comment:2 by zigiDev@…, 17 years ago

That is weird! :)

comment:3 by Chris Beaven, 17 years ago

Triage Stage: Unreviewed → 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 by Adrian Holovaty, 17 years ago

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 by zigiDev@…, 17 years ago

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 by scott@…, 17 years ago

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:8 by Simon G. <dev@…>, 17 years ago

Triage Stage: Accepted → 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 by Adrian Holovaty, 17 years ago

Resolution: → fixed
Status: new → closed

(In [5882]) Fixed #5030 -- Removed 'COUNT()' from Model.save() 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