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