UPSERT on partition

Started by Fujii Masaoover 10 years ago11 messages
#1Fujii Masao
masao.fujii@gmail.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Andres Freund
andres@anarazel.de
In reply to: Fujii Masao (#1)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#2)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#3)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Peter Geoghegan
pg@heroku.com
In reply to: Robert Haas (#3)
Re: UPSERT on partition

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]/messages/by-id/CA+TgmoZAVXMwOeBPK4ASZonuwUr3QPSWWk6XetXMcA+8H7Cd8Q@mail.gmail.com.

Even if we added what was discussed as a minimal and practical way of
exposing this [2]/messages/by-id/CAM3SWZRvjyu8nnvw_JHeXx4YMQ9XaA7u0GEtLbCgym0oEAn_7Q@mail.gmail.com -- Peter Geoghegan, 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]: /messages/by-id/CA+TgmoZAVXMwOeBPK4ASZonuwUr3QPSWWk6XetXMcA+8H7Cd8Q@mail.gmail.com
[2]: /messages/by-id/CAM3SWZRvjyu8nnvw_JHeXx4YMQ9XaA7u0GEtLbCgym0oEAn_7Q@mail.gmail.com -- Peter Geoghegan
--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Peter Geoghegan
pg@heroku.com
In reply to: Fujii Masao (#1)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Peter Geoghegan
pg@heroku.com
In reply to: Peter Geoghegan (#6)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Peter Geoghegan (#5)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Peter Geoghegan (#7)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Simon Riggs
simon@2ndQuadrant.com
In reply to: Fujii Masao (#1)
Re: UPSERT on partition

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/
<http://www.2ndquadrant.com/&gt;
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#11Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#8)
Re: UPSERT on partition

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers