Re: Partitioning vs ON CONFLICT

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: "Shinoda, Noriyoshi" <noriyoshi(dot)shinoda(at)hpe(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Thom Brown <thom(at)linux(dot)com>
Subject: Re: Partitioning vs ON CONFLICT
Date: 2017-03-31 20:47:28
Message-ID: CA+Tgmoaq=kdFwB2Bmwc+Mkk34UmaPpfTCfhNnj=_wHQGumfTwQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 30, 2017 at 5:54 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> I found out why the crash occurs, but while I was trying to fix it, I
> started growing doubtful about the way this is being handled currently.
>
> Patch to fix the crash would be to pass 'true' instead of 'false' for
> speculative when ExecSetupPartitionTupleRouting() calls ExecOpenIndices()
> on leaf partitions. That will initialize the information needed when
> ExecInsert() wants check for conflicts using the constraint-enforcing
> indexes. If we do initialize the speculative insertion info (which will
> fix the crash), ExecCheckIndexConstraints() will be called on a given leaf
> partition's index to check if there is any conflict. But since the insert
> was performed on the root table, conflicts should be checked across all
> the partitions, which won't be the case. Even though the action is
> NOTHING, the check for conflicts still uses only that one leaf partition's
> index, which seems insufficient.
>
> Commit 8355a011a0 enabled specifying ON CONFLICT DO NOTHING on when
> inserting into a partitioned root table, but given the above, I think we
> might need to reconsider it.

It seems obvious in retrospect that the commit in question was not
quite up to the mark, considering that it didn't even update the
comment here:

/*
* Open partition indices (remember we do not support ON CONFLICT in
* case of partitioned tables, so we do not need support information
* for speculative insertion)
*/

Part of the question here is definitional. Peter rightly pointed out
upthread that we support INSERT .. ON CONFLICT in an inheritance
situation, but that is different, because it infers whether there is a
conflict in the particular child into which you are trying to insert,
not whether there is a conflict across the whole hierarchy. More or
less by definition, trying to insert into the room of the partitioning
hierarchy is a different beast: it should consider uniqueness across
the whole hierarchy in determining whether there is a conflict and, as
Simon pointed out in the second email on the thread, we lack a
mechanism to do that. If somebody wants to consider only conflicts
within a specific partition, they can use INSERT .. ON CONFLICT with
the name of that partition, and that'll work fine; if they target the
parent, that should really apply globally to the hierarchy, which we
can't support.

So I think you (Amit) are right, and we should revert this commit. We
can try again to make this work in a future release once we've had a
chance to think about it some more.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-03-31 20:57:53 Re: Foreign tables don't enforce the partition constraint
Previous Message Robert Haas 2017-03-31 20:29:22 Re: Partitioned tables and relfilenode