On 9 June 2018 at 04:52, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Truth is, the more I look at this, the more I think it should be done in
> the way Amit Langote was proposing: do away with the extra function, and
> check all those conditions inside ExecConstraints itself. We can add a
> new boolean tupleRouting argument, which I think is the only missing bit.
As far as I see it what Amit Langote is proposing is to shift the
needless additional partition check from INSERT to UPDATE. I've tested
his patch and confirmed that this is the case. Amit has even written:
On 7 June 2018 at 21:37, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> There is one twist though. It may happen that ExecUpdate ends up
> performing tuple routing for one row and thus will have ri_PartitionRoot
> set. For another row, tuple routing may not be needed as the updated
> version of the row satisfies the partition constraint and now because
> ri_PartitionRoot has been set, ExecConstraint would end up performing the
> partition constraint check redundantly if trig_insert_before_row is true.
I'm really struggling to understand why this is being suggested at all.
Of course, that could be fixed by adding something like "bool
isinsert" to ExecConstraints(), so that it does not do the needless
check on UPDATE, but I'm strongly against the reduction in modularity.
I quite like ExecConstraints() the way it is.
FWIW, my test case that shows Amit's patch is wrong is:
drop table listp;
create table listp (a int, b int) partition by list(a);
create table listp12 partition of listp for values in(1,2);
create table listp34 partition of listp for values in(3,4);
create or replace function listp_insert_trigger() returns trigger as
$$ begin raise notice '%', NEW.a; return NEW; end; $$ language
plpgsql;
create trigger listp12_insert before insert on listp12 for each row
execute procedure listp_insert_Trigger();
insert into listp values(1,2);
update listp set b=1;
stick a breakpoint in ExecPartitionCheck() and watch it being hit
twice on the update. Revert Amit's patch and you'll hit it once.
I'm fine with my v3 patch, or your idea with the function in v5, but
with respect to Amit L, I'm strongly against his patch on the grounds
that it just moves the problem somewhere else.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services