From a90decd69a42bebdb6e07c8268686c0500f8c48e Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 16 Apr 2018 17:31:42 +0900 Subject: [PATCH v2] Couple of fixes for ExecInitPartitionInfo First, avoid repeated calling of map_partition_varattnos. To do that, generate the rootrel -> partrel attribute conversion map ourselves just once and call map_variable_attnos() directly with it. Second, support conversion of whole-row variables that appear in ON CONFLICT expressions. Add relevant test. --- src/backend/executor/execPartition.c | 88 ++++++++++++++++++++------- src/test/regress/expected/insert_conflict.out | 16 +++++ src/test/regress/sql/insert_conflict.sql | 14 +++++ 3 files changed, 97 insertions(+), 21 deletions(-) diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 218645d43b..1727e111bb 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -24,6 +24,7 @@ #include "nodes/makefuncs.h" #include "partitioning/partbounds.h" #include "partitioning/partprune.h" +#include "rewrite/rewriteManip.h" #include "utils/lsyscache.h" #include "utils/partcache.h" #include "utils/rel.h" @@ -309,6 +310,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, partrel; ResultRelInfo *leaf_part_rri; MemoryContext oldContext; + AttrNumber *part_attnos = NULL; + bool found_whole_row; /* * We locked all the partitions in ExecSetupPartitionTupleRouting @@ -397,8 +400,19 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, /* * Convert Vars in it to contain this partition's attribute numbers. */ - wcoList = map_partition_varattnos(wcoList, firstVarno, - partrel, firstResultRel, NULL); + part_attnos = + convert_tuples_by_name_map(RelationGetDescr(partrel), + RelationGetDescr(firstResultRel), + gettext_noop("could not convert row type")); + wcoList = (List *) + map_variable_attnos((Node *) wcoList, + firstVarno, 0, + part_attnos, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + foreach(ll, wcoList) { WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll)); @@ -446,9 +460,20 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, /* * Convert Vars in it to contain this partition's attribute numbers. */ - returningList = map_partition_varattnos(returningList, firstVarno, - partrel, firstResultRel, - NULL); + if (part_attnos == NULL) + part_attnos = + convert_tuples_by_name_map(RelationGetDescr(partrel), + RelationGetDescr(firstResultRel), + gettext_noop("could not convert row type")); + returningList = (List *) + map_variable_attnos((Node *) returningList, + firstVarno, 0, + part_attnos, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + leaf_part_rri->ri_returningList = returningList; /* @@ -549,14 +574,27 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * target relation (firstVarno). */ onconflset = (List *) copyObject((Node *) node->onConflictSet); - onconflset = - map_partition_varattnos(onconflset, INNER_VAR, partrel, - firstResultRel, &found_whole_row); - Assert(!found_whole_row); - onconflset = - map_partition_varattnos(onconflset, firstVarno, partrel, - firstResultRel, &found_whole_row); - Assert(!found_whole_row); + if (part_attnos == NULL) + part_attnos = + convert_tuples_by_name_map(RelationGetDescr(partrel), + RelationGetDescr(firstResultRel), + gettext_noop("could not convert row type")); + onconflset = (List *) + map_variable_attnos((Node *) onconflset, + INNER_VAR, 0, + part_attnos, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + onconflset = (List *) + map_variable_attnos((Node *) onconflset, + firstVarno, 0, + part_attnos, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ /* Finally, adjust this tlist to match the partition. */ onconflset = adjust_partition_tlist(onconflset, map); @@ -587,14 +625,22 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, List *clause; clause = copyObject((List *) node->onConflictWhere); - clause = map_partition_varattnos(clause, INNER_VAR, - partrel, firstResultRel, - &found_whole_row); - Assert(!found_whole_row); - clause = map_partition_varattnos(clause, firstVarno, - partrel, firstResultRel, - &found_whole_row); - Assert(!found_whole_row); + clause = (List *) + map_variable_attnos((Node *) clause, + INNER_VAR, 0, + part_attnos, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + clause = (List *) + map_variable_attnos((Node *) clause, + firstVarno, 0, + part_attnos, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ leaf_part_rri->ri_onConflict->oc_WhereClause = ExecInitQual((List *) clause, &mtstate->ps); } diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 2d7061fa1b..66ca1839bc 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -884,4 +884,20 @@ insert into parted_conflict values (40, 'forty'); insert into parted_conflict_1 values (40, 'cuarenta') on conflict (a) do update set b = excluded.b; ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification +-- test whole-row Vars in ON CONFLICT expressions +create unique index on parted_conflict (a, b); +alter table parted_conflict add c int; +truncate parted_conflict; +insert into parted_conflict values (50, 'cuarenta', 1); +insert into parted_conflict values (50, 'cuarenta', 2) + on conflict (a, b) do update set (a, b, c) = row(excluded.*) + where parted_conflict = (50, text 'cuarenta', 1) and + excluded = (50, text 'cuarenta', 2); +-- should see (50, 'cuarenta', 2) +select * from parted_conflict order by a; + a | b | c +----+----------+--- + 50 | cuarenta | 2 +(1 row) + drop table parted_conflict; diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index 6c50fd61eb..fb30530a54 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -558,4 +558,18 @@ create table parted_conflict_1_1 partition of parted_conflict_1 for values from insert into parted_conflict values (40, 'forty'); insert into parted_conflict_1 values (40, 'cuarenta') on conflict (a) do update set b = excluded.b; + +-- test whole-row Vars in ON CONFLICT expressions +create unique index on parted_conflict (a, b); +alter table parted_conflict add c int; +truncate parted_conflict; +insert into parted_conflict values (50, 'cuarenta', 1); +insert into parted_conflict values (50, 'cuarenta', 2) + on conflict (a, b) do update set (a, b, c) = row(excluded.*) + where parted_conflict = (50, text 'cuarenta', 1) and + excluded = (50, text 'cuarenta', 2); + +-- should see (50, 'cuarenta', 2) +select * from parted_conflict order by a; + drop table parted_conflict; -- 2.11.0