Re: ON CONFLICT DO UPDATE for partitioned tables

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Andres Freund <andres(at)anarazel(dot)de>, 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-04-10 02:56:13
Message-ID: cd9aa0e9-27e1-eb59-498a-91a091d9f4ba@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018/03/27 13:27, Amit Langote wrote:
> On 2018/03/26 23:20, Alvaro Herrera wrote:
>> The one thing I wasn't terribly in love with is the four calls to
>> map_partition_varattnos(), creating the attribute map four times ... but
>> we already have it in the TupleConversionMap, no? Looks like we could
>> save a bunch of work there.
>
> Hmm, actually we can't use that map, assuming you're talking about the
> following map:
>
> TupleConversionMap *map = proute->parent_child_tupconv_maps[partidx];
>
> We can only use that to tell if we need converting expressions (as we
> currently do), but it cannot be used to actually convert the expressions.
> The map in question is for use by do_convert_tuple(), not to map varattnos
> in Vars using map_variable_attnos().
>
> But it's definitely a bit undesirable to have various
> map_partition_varattnos() calls within ExecInitPartitionInfo() to
> initialize the same information (the map) multiple times.

I will try to think of doing something about this later this week.

>> And a final item is: can we have a whole-row expression in the clauses?
>> We currently don't handle those either, not even to throw an error.
>> [figures a test case] ... and now that I test it, it does crash!
>>
>> create table part (a int primary key, b text) partition by range (a);
>> create table part1 (b text, a int not null);
>> alter table part attach partition part1 for values from (1) to (1000);
>> insert into part values (1, 'two') on conflict (a)
>> do update set b = format('%s (was %s)', excluded.b, part.b)
>> where part.* *<> (1, text 'two');
>>
>> I think this means we should definitely handle found_whole_row. (If you
>> create part1 in the normal way, it works as you'd expect.)
>
> I agree. That means we simply remove the Assert after the
> map_partition_varattnos call.
>
>> I'm going to close a few other things first, then come back to this.
>
> Attached find a patch to fix the whole-row expression issue. I added your
> test to insert_conflict.sql.

Adding this to the open items list.

Thanks,
Amit

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-04-10 02:59:43 Re: pgsql: Merge catalog/pg_foo_fn.h headers back into pg_foo.h headers.
Previous Message Peter Geoghegan 2018-04-10 02:30:14 Re: WIP: Covering + unique indexes.