From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Geoghegan <pg(at)bowt(dot)ie> |
Subject: | Re: ON CONFLICT DO UPDATE for partitioned tables |
Date: | 2018-03-20 04:30:26 |
Message-ID: | f484764f-fe76-eb56-6e4d-889041d62ad4@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Fujita-san,
On 2018/03/19 21:59, Etsuro Fujita wrote:
> (2018/03/18 13:17), Alvaro Herrera wrote:
>> Alvaro Herrera wrote:
>> The only thing that I remain unhappy about this patch is the whole
>> adjust_and_expand_partition_tlist() thing. I fear we may be doing
>> redundant and/or misplaced work. I'll look into it next week.
>
> I'm still reviewing the patches, but I really agree on that point. As
> Pavan mentioned upthread, the onConflictSet tlist for the root parent,
> from which we create a translated onConflictSet tlist for a partition,
> would have already been processed by expand_targetlist() to contain all
> missing columns as well, so I think we could create the tlist for the
> partition by simply re-ordering the expression-converted tlist (ie,
> conv_setproj) based on the conversion map for the partition. The Attached
> defines a function for that, which could be called, instead of calling
> adjust_and_expand_partition_tlist(). This would allow us to get rid of
> planner changes from the patches. Maybe I'm missing something, though.
Thanks for the patch. I can confirm your proposed
adjust_onconflictset_tlist() is enough to replace adjust_inherited_tlist()
+ expand_targetlist() combination (aka
adjust_and_expand_partition_tlist()), thus rendering the planner changes
in this patch unnecessary. I tested it with a partition tree involving
partitions of varying attribute numbers (dropped columns included) and it
seems to work as expected (as also exercised in regression tests) as shown
below.
Partitioned table p has partitions p1, p2, p3, p4, and p5 whose attributes
look like this; shown as (colname: attnum, ...).
p: (a: 1, b: 4)
p1: (a: 1, b: 4)
p2: (a: 2, b: 4)
p3: (a: 1, b: 3)
p4: (a: 3, b: 8)
p5: (a: 1, b: 5)
You may notice that all partitions but p1 will have a tuple conversion map
and hence will undergo adjust_onconflictset_tlist() treatment.
insert into p values (1, 'a') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 1
insert into p values (1, 'b') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 1
insert into p values (1, 'c') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 0
insert into p values (1) on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 0
insert into p values (2, 'a') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 1
insert into p values (2, 'b') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 1
insert into p values (2, 'c') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 0
insert into p values (5, 'a') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 1
insert into p values (5, 'b') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 1
insert into p values (5, 'c') on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 0
insert into p values (5) on conflict (a) do update set b = excluded.b
where excluded.b = 'b';
INSERT 0 0
select tableoid::regclass, * from p;
tableoid | a | b
----------+---+---
p1 | 1 | b
p2 | 2 | b
p5 | 5 | b
(3 rows)
I have incorporated your patch in the main patch after updating the
comments a bit. Also, now that 6666ee49f49 is in [1], the transition
table related tests I proposed yesterday pass nicely. Instead of posting
as a separate patch, I have merged it with the main patch. So now that
planner refactoring is unnecessary, attached is just one patch.
Thanks,
Amit
[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6666ee49f49
Attachment | Content-Type | Size |
---|---|---|
v6-0001-Fix-ON-CONFLICT-to-work-with-partitioned-tables.patch | text/plain | 43.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2018-03-20 04:36:00 | Re: [HACKERS] plpgsql - additional extra checks |
Previous Message | Michael Paquier | 2018-03-20 04:28:18 | Re: Trigger file behavior with the standby |