Thanks Jeevan for looking at this. See comments below.
On 2017/08/02 19:04, Jeevan Ladhe wrote:
> On Wed, Aug 2, 2017 at 10:26 AM, Amit Langote wrote:
>> The patch's job is simple:
>>
>> - Remove the check in the parser that causes an error the moment the
>> ON CONFLICT clause is found.
>>
>> - Fix leaf partition ResultRelInfo initialization code so that the call
>> ExecOpenIndices() specifies 'true' for speculative, so that the
>> information necessary for conflict checking will be initialized in the
>> leaf partition's ResultRelInfo
>
> I applied the patch on latest master sources and the patch applies cleanly.
> The documentation is built without errors.
>
> We do not support following syntax for 'do nothing':
>
> postgres=# insert into parted_conflict_test values (1, 'a') on conflict (b)
> do nothing;
> ERROR: there is no unique or exclusion constraint matching the ON CONFLICT
> specification
To nitpick, the above is not a syntax error; we *do* support the syntax to
specify conflict target even when the conflict action is DO NOTHING. The
error is emitted by the planner when it fails to find the index to cover
column 'b' that's specified as the conflict target.
> This limitation is because we do not support unique index on partitioned
> table.
Yes.
> But, in that sense the following snippet of the documentation seems
> misleading:
>
> + 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.
> May be the last sentence can be rephrased as below:
>
> "On the other hand, specifying <literal>DO NOTHING</literal> without target
> as
> an alternative action works fine."
I have updated the text.
> Other than this patch looks good to me.
Updated patch attached.
Thanks,
Amit
[1]
https://www.postgresql.org/docs/devel/static/sql-insert.html#sql-on-conflict
From ec32e99310c034b26db1c5478ed38641150a7aec 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, 35 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index b05a9c2150..23b0c42dba 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3276,9 +3276,15 @@ 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
+ will cause an error if the conflict target is specified (see
+ <xref linkend="sql-on-conflict"> for more details on how the clause
+ works). That means it's not possible to specify
+ <literal>DO UPDATE</literal> as the alternative action, because
+ specifying the conflict target is mandatory in that case. On the other
+ hand, specifying <literal>DO NOTHING</literal> as the alternative action
+ works fine provided the conflict target is not specified. In that case,
+ unique constraints (or exclusion constraints) of the individual leaf
+ partitions are considered, not those across the whole partitioning
hierarchy.
</para>
</listitem>
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index c11aa4fe21..b313ad1efa 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -3300,13 +3300,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 4fb793cfbf..73b99c2585 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -847,16 +847,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
