pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedFrom: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>
Cc: Pavan Deolasee <pavan.deolasee@gmail.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Peter Geoghegan <pg@bowt.ie>
Subject: Re: ON CONFLICT DO UPDATE for partitioned tables
Date: Mon, 19 Mar 2018 21:59:01 +0900
Message-ID: <5AAFB415.8070305@lab.ntt.co.jp> (raw)
In-Reply-To: <20180318041715.xvn6lwiv23pcdbny@alvherre.pgsql>
References: <20180318041715.xvn6lwiv23pcdbny@alvherre.pgsql>
(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.
Best regards,
Etsuro Fujita
Attachments:
[text/x-diff] simplify-tlist-adjustment.patch (3.3K, ../5AAFB415.8070305@lab.ntt.co.jp/2-simplify-tlist-adjustment.patch)
download | inline diff:
*** src/backend/executor/execPartition.c.org 2018-03-19 21:32:52.000000000 +0900
--- src/backend/executor/execPartition.c 2018-03-19 21:44:17.000000000 +0900
***************
*** 15,24 ****
--- 15,26 ----
#include "postgres.h"
#include "catalog/pg_inherits_fn.h"
+ #include "catalog/pg_type.h"
#include "executor/execPartition.h"
#include "executor/executor.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
+ #include "nodes/makefuncs.h"
#include "optimizer/prep.h"
#include "utils/lsyscache.h"
#include "utils/rls.h"
***************
*** 37,42 ****
--- 39,45 ----
Datum *values,
bool *isnull,
int maxfieldlen);
+ static List *adjust_onconflictset_tlist(List *tlist, TupleConversionMap *map);
/*
* ExecSetupPartitionTupleRouting - sets up information needed during
***************
*** 554,565 ****
firstVarno, partrel,
firstResultRel, NULL);
! conv_setproj =
! adjust_and_expand_partition_tlist(RelationGetDescr(firstResultRel),
! RelationGetDescr(partrel),
! RelationGetRelationName(partrel),
! firstVarno,
! conv_setproj);
tupDesc = ExecTypeFromTL(conv_setproj, partrelDesc->tdhasoid);
part_conflproj_slot = ExecInitExtraTupleSlot(mtstate->ps.state,
--- 557,563 ----
firstVarno, partrel,
firstResultRel, NULL);
! conv_setproj = adjust_onconflictset_tlist(conv_setproj, map);
tupDesc = ExecTypeFromTL(conv_setproj, partrelDesc->tdhasoid);
part_conflproj_slot = ExecInitExtraTupleSlot(mtstate->ps.state,
***************
*** 1091,1093 ****
--- 1089,1153 ----
return buf.data;
}
+
+ /*
+ * Adjust the targetlist entries of a translated ON CONFLICT UPDATE operation
+ *
+ * The expressions have already been fixed, but we need to re-order the tlist
+ * so that the target resnos match the child table.
+ *
+ * The given tlist has already been through expression_tree_mutator;
+ * therefore the TargetEntry nodes are fresh copies that it's okay to
+ * scribble on.
+ */
+ static List *
+ adjust_onconflictset_tlist(List *tlist, TupleConversionMap *map)
+ {
+ List *new_tlist = NIL;
+ TupleDesc tupdesc = map->outdesc;
+ AttrNumber *attrMap = map->attrMap;
+ int numattrs = tupdesc->natts;
+ int attrno;
+
+ for (attrno = 1; attrno <= numattrs; attrno++)
+ {
+ Form_pg_attribute att_tup = TupleDescAttr(tupdesc, attrno - 1);
+ TargetEntry *tle;
+
+ if (attrMap[attrno - 1] != 0)
+ {
+ Assert(!att_tup->attisdropped);
+
+ /* Get the corresponding tlist entry from the given tlist */
+ tle = (TargetEntry *) list_nth(tlist, attrMap[attrno - 1] - 1);
+
+ /* Get the resno right */
+ if (tle->resno != attrno)
+ tle->resno = attrno;
+ }
+ else
+ {
+ Node *expr;
+
+ Assert(att_tup->attisdropped);
+
+ /* Insert NULL for dropped column */
+ expr = (Node *) makeConst(INT4OID,
+ -1,
+ InvalidOid,
+ sizeof(int32),
+ (Datum) 0,
+ true, /* isnull */
+ true /* byval */ );
+
+ tle = makeTargetEntry((Expr *) expr,
+ attrno,
+ pstrdup(NameStr(att_tup->attname)),
+ false);
+ }
+
+ new_tlist = lappend(new_tlist, tle);
+ }
+
+ return new_tlist;
+ }
view thread (53+ messages) latest in thread
Message-ID: <5AAFB415.8070305@lab.ntt.co.jp>
Permalink: ../5AAFB415.8070305@lab.ntt.co.jp/
Also on: postgresql.org/message-id/5AAFB415.8070305@lab.ntt.co.jp
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: fujita.etsuro@lab.ntt.co.jp, alvherre@alvh.no-ip.org, andres@anarazel.de, pavan.deolasee@gmail.com, Langote_Amit_f8@lab.ntt.co.jp, pg@bowt.ie
Subject: Re: ON CONFLICT DO UPDATE for partitioned tables
In-Reply-To: <5AAFB415.8070305@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox