On 2017/04/01 6:44, Robert Haas wrote:
> On Fri, Mar 31, 2017 at 5:33 PM, Peter Geoghegan <p...@bowt.ie> wrote:
>> In my opinion, for the very limited ON CONFLICT DO NOTHING + no
>> inference specification case, the implementation should not care about
>> the presence or absence of unique indexes within or across partitions.
>
> Hmm. That's an interesting point. The documentation says:
>
> ON CONFLICT can be used to specify an alternative action to raising a
> unique constraint or exclusion constraint violation error.
>
> And, indeed, you could get an unique constraint or exclusion error
> because of an index on the child even though it's not global to the
> partitioning hierarchy. So maybe we can support this after all, but
Oh, I see. Thanks to both of you for the explanations.
Users will be aware that a partitioned parent does not allow defining
unique/exclusion constraints that span partitions, so also that any
conflicts detected by INSERT .. ON CONFLICT DO NOTHING are only at the
level of individual leaf partitions, if there indeed are unique/exclusion
indexes defined on them.
So, if we have:
create table parent (a char, b int) partition by list (a);
create table part_a partition of parent (b unique) for values in ('a');
create table part_b partition of parent (b unique) for values in ('b');
Session-1 and session-2 both perform:
insert into parent values ('a', 1) on conflict do nothing;
Also, session-3 and session-4 both perform (possibly concurrently with
session-1 and session-2):
insert into parent values ('b', 1) on conflict do nothing;
One of session-1 or session-2 succeeds in inserting ('a', 1) into part_a
and the other does "nothing" when it finds it there already. Similarly,
one of session-3 and session-4 succeeds in inserting ('b', 1) into part_b
and the other does "nothing". If on conflict do nothing clause wasn't
there, the other session will error out. If there had not been those
unique indexes, part_a will have two instances of ('a', 1) and part_b will
have two of ('b', 1), irrespective of whether the on conflict do nothing
clause was specified.
Since nowhere has the user asked to ensure unique(b) across partitions by
defining the same on parent, this seems just fine. But one question to
ask may be whether that will *always* be the case? That is, will we take
ON CONFLICT DO NOTHING without the conflict target specification to mean
checking for conflicts on the individual leaf partition level, even in the
future when we may have global constraints?
> having messed it up once, I'm inclined to think we should postpone
> this to v11, think it over some more, and try to make sure that our
> second try doesn't crash...
Just in case, here is a patch that (re-)implements the limited support we
previously tried to implement in the commit that was just reverted.
Documentation is improved from the last version considering this
discussion and also the source code comments.
Thanks,
Amit
>From ec732ac5d8fd87f657f51a464072eb57d5f888dd Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 3 Apr 2017 19:13:38 +0900
Subject: [PATCH] Allow ON CONFLICT DO NOTHING on partitioned tables
ON CONFLICT .. DO UPDATE still doesn't work, because it requires
specifying the conflict target. DO NOTHING doesn't require it,
but the executor will check for conflicts within only a given
leaf partitions, if relevant constraints exist.
Specifying the conflict target makes the planner look for the
required indexes on the parent table, which are not allowed, so an
error will always be reported in that case.
---
doc/src/sgml/ddl.sgml | 12 ++++++++----
src/backend/executor/execMain.c | 10 ++++++----
src/backend/parser/analyze.c | 8 --------
src/test/regress/expected/insert_conflict.out | 10 ++++++++++
src/test/regress/sql/insert_conflict.sql | 10 ++++++++++
5 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 5109778196..478d7171cd 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3268,10 +3268,14 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
<listitem>
<para>
Using the <literal>ON CONFLICT</literal> clause with partitioned tables
- will cause an error, because unique or exclusion constraints can only be
- created on individual partitions. There is no support for enforcing
- uniqueness (or an exclusion constraint) across an entire partitioning
- hierarchy.
+ will cause an error if the conflict target is specified (see
+ <xref linkend="sql-insert"> for more details). That means it's not
+ possible to specify <literal>DO UPDATE</literal> as the alternative
+ action, because it requires the conflict target to be specified.
+ On the other hand, specifying <literal>DO NOTHING</literal> as the
+ alternative action works fine. Note that in the latter case, a unique
+ constraint (or an exclusion constraint) of the individual leaf
+ partitions is considered, not across the whole partitioning hierarchy.
</para>
</listitem>
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 920b12072f..dd691d6bc9 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -3201,13 +3201,15 @@ ExecSetupPartitionTupleRouting(Relation rel,
0);
/*
- * Open partition indices (remember we do not support ON CONFLICT in
- * case of partitioned tables, so we do not need support information
- * for speculative insertion)
+ * Open partition indices. The user may have asked to check for
+ * conflicts within this leaf partition and do "nothing" instead of
+ * throwing an error. Be prepared in that case by initializing the
+ * index information needed by ExecInsert() to perform speculative
+ * insertions.
*/
if (leaf_part_rri->ri_RelationDesc->rd_rel->relhasindex &&
leaf_part_rri->ri_IndexRelationDescs == NULL)
- ExecOpenIndices(leaf_part_rri, false);
+ ExecOpenIndices(leaf_part_rri, true);
leaf_part_rri++;
i++;
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 811fccaec9..a06b41098b 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -845,16 +845,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
/* Process ON CONFLICT, if any. */
if (stmt->onConflictClause)
- {
- /* Bail out if target relation is partitioned table */
- if (pstate->p_target_rangetblentry->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("ON CONFLICT clause is not supported with partitioned tables")));
-
qry->onConflict = transformOnConflictClause(pstate,
stmt->onConflictClause);
- }
/*
* If we have a RETURNING clause, we need to add the target relation to
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 8d005fddd4..617b0c0ed2 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -786,3 +786,13 @@ select * from selfconflict;
(3 rows)
drop table selfconflict;
+-- check that the following works:
+-- insert into partitioned_table on conflict do nothing
+create table parted_conflict_test (a int, b char) partition by list (a);
+create table parted_conflict_test_1 partition of parted_conflict_test (b unique) for values in (1);
+insert into parted_conflict_test values (1, 'a') on conflict do nothing;
+insert into parted_conflict_test values (1, 'a') on conflict do nothing;
+-- however, on conflict do update is not supported yet
+insert into parted_conflict_test values (1) on conflict (b) do update set a = excluded.a;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+drop table parted_conflict_test;
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index df3a9b59b5..6a4495865f 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -471,3 +471,13 @@ commit;
select * from selfconflict;
drop table selfconflict;
+
+-- check that the following works:
+-- insert into partitioned_table on conflict do nothing
+create table parted_conflict_test (a int, b char) partition by list (a);
+create table parted_conflict_test_1 partition of parted_conflict_test (b unique) for values in (1);
+insert into parted_conflict_test values (1, 'a') on conflict do nothing;
+insert into parted_conflict_test values (1, 'a') on conflict do nothing;
+-- however, on conflict do update is not supported yet
+insert into parted_conflict_test values (1) on conflict (b) do update set a = excluded.a;
+drop table parted_conflict_test;
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
