Thread: UPSERT on partition
Hi,
INSERT ON CONFLICT DO UPDATE doesn't seem to work on the current partitioning
mechanism. For example, in the following SQL commands, the last UPSERT command
would fail with an error. The error message is
ERROR: duplicate key value violates unique constraint "hoge_20150601_pkey" DETAIL: Key (col1)=(2015-06-01
10:30:00)already exists. CONTEXT: SQL statement "INSERT INTO hoge_20150601 VALUES (($1).*)" PL/pgSQL function
hoge_insert_trigger()line 6 at EXECUTE statement
------------------------------------------------------------------------------------
CREATE TABLE hoge (col1 TIMESTAMP PRIMARY KEY, col2 INT);
CREATE OR REPLACE FUNCTION hoge_insert_trigger () RETURNS trigger AS
$$ DECLARE part TEXT; BEGIN
part:= 'hoge_' || to_char(new.col1,
'YYYYMMDD'); EXECUTE 'INSERT INTO ' || part || '
VALUES (($1).*)' USING new; RETURN NULL; END;
$$
LANGUAGE plpgsql;
CREATE TRIGGER insert_hoge_trigger BEFORE INSERT ON hoge FOR EACH ROW EXECUTE PROCEDURE
hoge_insert_trigger();
CREATE TABLE hoge_20150601 ( LIKE hoge INCLUDING INDEXES INCLUDING DEFAULTS
INCLUDING CONSTRAINTS, CHECK ('2015-06-01 00:00:00' <= col1 AND col1 < '2015-06-02 00:00:00')
)
INHERITS (hoge);
CREATE TABLE hoge_20150602 ( LIKE hoge INCLUDING INDEXES INCLUDING DEFAULTS
INCLUDING CONSTRAINTS, CHECK ('2015-06-02 00:00:00' <= col1 AND col1 < '2015-06-03 00:00:00')
)
INHERITS (hoge);
INSERT INTO hoge VALUES ('2015-06-01 10:30:00', 1234) ON CONFLICT (col1) DO UPDATE SET col2 = EXCLUDED.col2;
INSERT INTO hoge VALUES ('2015-06-01 10:30:00', 1234) ON CONFLICT (col1) DO UPDATE SET col2 = EXCLUDED.col2;
------------------------------------------------------------------------------------
How should we treat this problem for 9.5? If we want to fix this problem
completely, probably we would need to make constraint_exclusion work with
even UPSERT. Which sounds difficult to do at least for 9.5. Any other idea?
Or we should just treat it as a limitation of UPSERT and add that document?
Thought?
Regards,
--
Fujii Masao
Hi, On 2015-06-24 23:05:45 +0900, Fujii Masao wrote: > INSERT ON CONFLICT DO UPDATE doesn't seem to work on the current partitioning > mechanism. For example, in the following SQL commands, the last UPSERT command > would fail with an error. The error message is I think that's pretty much inevitable without baking in touple routing into the core system and supporting unique-constraints that span partitions. In other words, I don't think this is upsert's fault. > Or we should just treat it as a limitation of UPSERT and add that document? I think we should (IIRC there's actually already something) rather document it as a limitation of the partitioning approach. Greetings, Andres Freund
On Wed, Jun 24, 2015 at 10:29 AM, Andres Freund <andres@anarazel.de> wrote: > On 2015-06-24 23:05:45 +0900, Fujii Masao wrote: >> INSERT ON CONFLICT DO UPDATE doesn't seem to work on the current partitioning >> mechanism. For example, in the following SQL commands, the last UPSERT command >> would fail with an error. The error message is > > I think that's pretty much inevitable without baking in touple routing > into the core system and supporting unique-constraints that span > partitions. In other words, I don't think this is upsert's fault. Is the root of the problem that the trigger is called for an INSERT .. ON CONFLICT statement but it turns that into a plain INSERT? Is there any way of writing a partitioning trigger that doesn't have that defect? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2015-06-24 10:38:38 -0400, Robert Haas wrote: > On Wed, Jun 24, 2015 at 10:29 AM, Andres Freund <andres@anarazel.de> wrote: > > On 2015-06-24 23:05:45 +0900, Fujii Masao wrote: > >> INSERT ON CONFLICT DO UPDATE doesn't seem to work on the current partitioning > >> mechanism. For example, in the following SQL commands, the last UPSERT command > >> would fail with an error. The error message is > > > > I think that's pretty much inevitable without baking in touple routing > > into the core system and supporting unique-constraints that span > > partitions. In other words, I don't think this is upsert's fault. > > Is the root of the problem that the trigger is called for an INSERT .. > ON CONFLICT statement but it turns that into a plain INSERT? If you'd not do that, you'd avoid errors when violating a unique key inside a partition, so it'd help a bit. But it'd not do anything sane when the conflict is actually *not* aligned with the partitioning schema, because we'll obviously only search for conflicts within the one table. Greetings, Andres Freund
On Wed, Jun 24, 2015 at 7:38 AM, Robert Haas <robertmhaas@gmail.com> wrote: > Is the root of the problem that the trigger is called for an INSERT .. > ON CONFLICT statement but it turns that into a plain INSERT? > > Is there any way of writing a partitioning trigger that doesn't have > that defect? We did discuss whether or not it was important to expose information to triggers sufficient to route + UPSERT tuples into inheritance children in an equivalent way (equivalent to an UPSERT into the inheritance parent). At the time, you seemed to think that it could wait [1]. Even if we added what was discussed as a minimal and practical way of exposing this [2], it would still be awkward for an INSERT trigger to examine the structure of the "UPDATE part" of the UPSERT -- there will be no *tuple* for that case (yet). Inheritance with triggers is a leaky abstraction, so this kind of thing is always awkward. Still, UPSERT has full support for *inheritance* -- that just doesn't help in this case. [1] http://www.postgresql.org/message-id/CA+TgmoZAVXMwOeBPK4ASZonuwUr3QPSWWk6XetXMcA+8H7Cd8Q@mail.gmail.com [2] http://www.postgresql.org/message-id/CAM3SWZRvjyu8nnvw_JHeXx4YMQ9XaA7u0GEtLbCgym0oEAn_7Q@mail.gmail.com -- Peter Geoghegan
On Wed, Jun 24, 2015 at 7:05 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > How should we treat this problem for 9.5? If we want to fix this problem > completely, probably we would need to make constraint_exclusion work with > even UPSERT. Which sounds difficult to do at least for 9.5. Any other idea? > Or we should just treat it as a limitation of UPSERT and add that document? I think that the real way to fix this is, as you say, to make it so that it isn't necessary in general to write trigger functions like this to make inheritance work. I can imagine the system rewriting an INSERT ... ON CONFLICT DO UPDATE query to target the correct child partition based on the values proposed for insertion, much like constraint exclusion. There are some problems with that idea, like what to do about BEFORE INSERT triggers changing those values (the values that we use to determine the appropriate child partition). These seem like problems that can be fixed. What I think cannot work is making the trigger inheritance pattern really robust with UPSERT. Sure, we can do a little bit, like expose basic information about whether the INSERT is ON CONFLICT DO NOTHING or ON CONFLICT DO UPDATE. But as you imply, it seems unwise to expect the user to do the rewriting themselves, within their trigger function. Maybe that's an argument against ever exposing even this basic information to the user. Thanks for testing. -- Peter Geoghegan
On Wed, Jun 24, 2015 at 11:02 AM, Peter Geoghegan <pg@heroku.com> wrote: > I think that the real way to fix this is, as you say, to make it so > that it isn't necessary in general to write trigger functions like > this to make inheritance work. Excuse me -- I mean to make *partitioning* work. -- Peter Geoghegan
Peter, On 2015-06-25 AM 02:35, Peter Geoghegan wrote: > > Inheritance with triggers is a leaky abstraction, so this kind of > thing is always awkward. Still, UPSERT has full support for > *inheritance* -- that just doesn't help in this case. > Could you clarify as to what UPSERT's support for inheritance entails? Thanks, Amit
On 6/24/15 1:03 PM, Peter Geoghegan wrote: > On Wed, Jun 24, 2015 at 11:02 AM, Peter Geoghegan <pg@heroku.com> wrote: >> I think that the real way to fix this is, as you say, to make it so >> that it isn't necessary in general to write trigger functions like >> this to make inheritance work. > > Excuse me -- I mean to make *partitioning* work. It occurs to me that we could run the BEFORE triggers and then pass the new tuple to a user-supplied function that returns a regclass. That would allow us to know exactly which child/partition the UPSERT should be attempted in. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Data in Trouble? Get it in Treble! http://BlueTreble.com
On 24 June 2015 at 15:05, Fujii Masao <masao.fujii@gmail.com> wrote:
--
How should we treat this problem for 9.5? If we want to fix this problem
completely, probably we would need to make constraint_exclusion work with
even UPSERT. Which sounds difficult to do at least for 9.5. Any other idea?
Or we should just treat it as a limitation of UPSERT and add that document?
+1
There are many problems that cannot be resolved for 9.5.
UPSERT works fine with tables with BRIN indexes.
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2015-06-25 AM 09:51, Amit Langote wrote: > > Peter, > > On 2015-06-25 AM 02:35, Peter Geoghegan wrote: >> >> Inheritance with triggers is a leaky abstraction, so this kind of >> thing is always awkward. Still, UPSERT has full support for >> *inheritance* -- that just doesn't help in this case. >> > > Could you clarify as to what UPSERT's support for inheritance entails? > Oh, I see that this stuff has been discussed (-hackers) and written about (wiki). I'll go read about it. I agree with Fujii-san's concern that any UPSERT on partition limitations given those of partitioning approach should be documented likewise, if not under INSERT/UPSERT, then under partitioning; not really sure which. Thanks, Amit