Re: ON CONFLICT DO UPDATE for partitioned tables

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Pavan Deolasee <pavan.deolasee@gmail.com>, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>, Peter Geoghegan <pg@bowt.ie>
Date: 2018-04-17T09:00:36Z
Lists: pgsql-hackers

Attachments

On 2018/04/17 16:45, Amit Langote wrote:
> Instead of doing this, I think we should try to make
> convert_tuples_by_name_map() a bit smarter by integrating the logic in
> convert_tuples_by_name() that's used conclude if no tuple conversion is
> necessary.  So, if it turns that the tuples descriptors passed to
> convert_tuples_by_name_map() contain the same number of attributes and the
> individual attributes are at the same positions, we signal to the caller
> that no conversion is necessary by returning NULL.
> 
> Attached find a patch that does that.
I just confirmed my hunch that this wouldn't somehow do the right thing
when the OID system column is involved.  Like this case:

create table parent (a int);
create table child () inherits (parent) with oids;
insert into parent values (1);
insert into child values (1);
analyze parent;
server closed the connection unexpectedly
	This probably means the server terminated abnormally
	before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

That's because, convert_tuples_by_name() that's called by
acquire_inherited_sample_rows() gets a TupleConversionMap whose attrMap is
set to NULL.  do_convert_tuple() may then try to access a member of such
NULL attrMap.  In this case, even if parent and child tables have same
user attributes, patched convert_tuples_by_name_map would return NULL, but
since their hasoids setting doesn't match, a TupleConversionMap is still
returned but has its attrMap set to NULL.  To fix that, I taught
do_convert_tuple() to ignore the map if NULL.  Also, free_conversion_map()
shouldn't try to free attrMap if it's NULL.

Attached updated patch.

Thanks,
Amit

Commits

  1. Remove quick path in ExecInitPartitionInfo for equal tupdescs

  2. Ignore whole-rows in INSERT/CONFLICT with partitioned tables

  3. Handle INSERT .. ON CONFLICT with partitioned tables

  4. Remove unnecessary members from ModifyTableState and ExecInsert