On 2017/02/17 14:50, Peter Geoghegan wrote:
> On Thu, Feb 16, 2017 at 9:27 PM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>> Attached patch fixes that. Thom, your example query should not error out
>> with the patch. As discussed here, DO UPDATE cannot be supported at the
>> moment.
>
> Maybe you should just let infer_arbiter_indexes() fail, rather than
> enforcing this directly. IIRC, that's what happens with
> inheritance-based partitioning.
That would be another way. The error message emitted by
infer_arbiter_indexes() would be:
ERROR: there is no unique or exclusion constraint matching the ON
CONFLICT specification
It does read better than what proposed patch makes
transformOnConflictClause() emit:
ERROR: ON CONFLICT ON UPDATE clause is not supported with partitioned tables
I updated the patch. Now it's reduced to simply removing the check in
transformInsertStmt() that prevented using *any* ON CONFLICT on
partitioned tables at all.
I don't however see why the error would *necessarily* occur in the case of
inheritance partitioning. I mean if inserts into the root table in an
inheritance hierarchy, it's still possible to ON CONFLICT DO UPDATE using
the unique index only on that table for inference, although that's what a
user would intend to do.
create table foo (a int, b int, unique (a));
create table foo_part (like foo including indexes) inherits (foo);
insert into foo values (1, 2);
-- the following still works
insert into foo values (1, 2)
on conflict (a) do update set b = excluded.b where excluded.a = 1;
insert into foo values (1, 2)
on conflict (a) do update set b = excluded.b where excluded.a = 1;
As the documentation about inheritance partitioning notes, that may not be
the behavior expected for partitioned tables:
<para>
<command>INSERT</command> statements with <literal>ON CONFLICT</>
clauses are unlikely to work as expected, as the <literal>ON CONFLICT</>
action is only taken in case of unique violations on the specified
target relation, not its child relations.
</para>
With partitioned tables, since it's not possible to create index
constraints on them, ON CONFLICT DO UPDATE simply won't work. So the
patch also updates the note in the document about partitioned tables and
ON CONFLICT.
Thanks,
Amit
>From 188f9e64402ce70f36e48274927fc6d5784319fa Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 17 Feb 2017 14:18:01 +0900
Subject: [PATCH] ON CONFLICT DO NOTHING should work with partitioned tables
Currently, a check in transformInsertStmt() prevents *any*
ON CONFLICT clause from being specified on a partitioned table,
even those specifying DO NOTHING as the alternative action. It
is harmless to allow those, so remove that check. It would still
not be possible to use DO UPDATE with partitioned table though,
because infer_arbiter_indexes() will eventually error out upon
failing to find a unique/exclusion constraint. Remember it is
not at the moment possible to create these constraints on
partitioned tables.
Adds a test and updates the note in document about using ON CONFLICT
with partitioned tables.
---
doc/src/sgml/ddl.sgml | 9 ++++++---
src/backend/parser/analyze.c | 8 --------
src/test/regress/expected/insert_conflict.out | 10 ++++++++++
src/test/regress/sql/insert_conflict.sql | 10 ++++++++++
4 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index f909242e4c..c99951a660 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3862,9 +3862,12 @@ ANALYZE measurement;
</para>
<para>
- <command>INSERT</command> statements with <literal>ON CONFLICT</>
- clause are currently not allowed on partitioned tables, that is,
- cause error when specified.
+ Using the <literal>ON CONFLICT</literal> clause with partitioned tables
+ will cause an error if <literal>DO UPDATE</literal> is specified as the
+ alternative action, because it requires specifying a unique or exclusion
+ constraint to determine if there is a conflict. Currently, it is not
+ possible to create indexes on partitioned tables required to implement
+ such constraints.
</para>
</listitem>
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f7659bb6b..a25a7c503a 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -843,16 +843,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..c90d381b34 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 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 not supported yet
+insert into parted_conflict_test values (1) on conflict (a) do update set b = excluded.b where excluded.a = 1;
+ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification
+drop table parted_conflict_test, parted_conflict_test_1;
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index df3a9b59b5..78bffc783d 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 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 not supported yet
+insert into parted_conflict_test values (1) on conflict (a) do update set b = excluded.b where excluded.a = 1;
+drop table parted_conflict_test, parted_conflict_test_1;
--
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
