Optimize Merge queries in Postgres driver
Crell - August 28, 2008 - 11:16
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | postgresql database |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
| Issue tags: | Performance |
Jump to:
Description
Currently Postgres does not have a Postgres-specific implementation of MERGE queries. Instead, we're falling back to the slow, 2-query mechanism in the base driver that has a small race condition in it. We therefore need someone to write a proper Postgres-specific implementation.
After talking to some Postgres gurus, we've determined that the correct way to do that is to write a custom Postgres function that we can then call. However, I know absolutely nothing about writing Postgres functions. If you do, please speak up and send in a patch! :-)
#1
Can you define the parameters for the patch? What are you going to pass the function? What do you expect it to return?
#2
The API to the merge query is already defined. key(), fields(), etc. I don't know enough about Postgres to say how such a Postgres function should work, but it needs to mimic the behavior of the method calls to the object.
#3
Merge queries are now documented here, too: http://drupal.org/node/310085
#4
At the bottom of the following page there is an example that might be a start for this.
http://www.postgresql.org/docs/current/static/plpgsql-control-structures...
And here's how it has been solved in the MySQL driver:
http://drupal.org/node/301501
HTH+ :)
#5
A few more references, taken from: http://wiki.postgresql.org/wiki/Todo
Another possible reference from the PostgreSQL manual:
http://www.postgresql.org/docs/7.4/interactive/rules-update.html
#6
Is there someone with the expertise to write and/or review a patch here? Seems like this could provide significant performance improvements.
#7
adding performance tag
#8
and also PostgresSQL sorry.
#9
I twitted about that this morning, and @jwpye suggested to use http://mbk.projects.postgresql.org/. It is not GPL but apparently covers our use case.
#10
Hum, scratch that, it does only work with simple conditions.
#11
Refiling...
#12
will a simple subquery do?
#13
I have no idea, will it? :-) Right now, Postgres uses the generic merge query which is non-atomic, because it's a SELECT then if-else for INSERT or UPDATE. I don't know enough Postgres to say how best to make it atomic. If it works and doesn't suck performance-wise, I'm cool with it. :-)
If it can't be done, well, OK, it can't be done. But I'd hate to leave that situation in there if we don't have to.
#14
:S err.... looks like not. So a merge query for PostgreSQL (and SQLite?) will do:
BEGIN;SELECT .....
INSERT ....
-- OR
UPDATE ....
COMMIT;
4 queries to the database. But I don't see a better solution in 8.3. The fact that there is a transaction makes it atomic though. So I'm happy with the current implementation.
#15
Hm, good point. So it is atomic, just not the fastest. I'm OK with that.
I'm bumping this to Drupal 8, in case someone wants to look into it again there, but for now it's not a blocker for D7.
#16
subscribe
#17
Right now db_merge() does not work for postgres at all
Why not just build a query like
$BODY$DECLARE sql text;
DECLARE sql2 text;
BEGIN
sql := 'UPDATE '|| quote_ident($1) ||' SET field1 = ' || $3 || ', field2 = ' || quote_literal($4) || ' WHERE PrimaryKey = ' || quote_literal($2) || ';';
EXECUTE sql;
BEGIN
sql2 := 'INSERT INTO '|| quote_ident($1) ||' (PrimaryKey, field1, field2) VALUES ('''|| $2 ||''' , '|| $3 ||' , '''|| $4 ||''');';
EXECUTE sql2;
EXCEPTION WHEN unique_violation THEN
END;
END;
$BODY$
#18
If db_merge() doesn't work, how is Postgres even functional at this point? :-) I dispute that claim.
I have no idea if that procedure would work or not. Josh?
#19
Postgres is broken now, details #636632: Decide on multiple database testing #13-14