On 2017/02/17 13:25, Peter Geoghegan wrote:
> On Thu, Feb 16, 2017 at 8:21 PM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>> would be working on a leaf partition chosen by tuple-routing after an
>> insert on a partitioned table. The leaf partitions can very well have a
>> unique index, which can be used for inference. The problem however is
>> that infer_arbiter_indexes() in the optimizer would be looking at the root
>> partitioned, which cannot yet have any indexes defined on them, let alone
>> unique indexes. When we develop a feature where defining an index on the
>> root partitioned table would create the same index on all the leaf
>> partitions and then extend it to support unique indexes, then we can
>> perhaps talk about supporting ON CONFLICT handing. Does that make sense?
>
> Yes, that makes sense, but I wasn't arguing that that should be
> possible today. I was arguing that when you don't spell out an
> arbiter, which ON CONFLICT DO NOTHING permits, then it should be
> possible for it to just work today -- infer_arbiter_indexes() will
> return immediately.
I see. It now seems that I should have realized the DO NOTHING action is
indeed supportable when I initially wrote the code that causes the current
error.
> This should be just like the old approach involving inheritance, in
> that that should be possible. No?
So we should error out only when the DO UPDATE conflict action is
requested. Because it will require specifying conflict_target, which it's
not possible to do in case of partitioned tables.
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.
Thanks,
Amit
>From 63f2c2d6c47300cb7f1a422a0a5d2697223f55e3 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
The DO NOTHING conflict action does not require one to specify
conflict_target, which would require arbiter indexes to be defined
on the table. So, only error out if DO UPDATE is requested as
the conflict action.
Adds the test as well.
---
src/backend/parser/analyze.c | 20 ++++++++++++--------
src/test/regress/expected/insert_conflict.out | 10 ++++++++++
src/test/regress/sql/insert_conflict.sql | 10 ++++++++++
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f7659bb6b..8e91f2f7c2 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
@@ -1010,6 +1002,18 @@ transformOnConflictClause(ParseState *pstate,
List *exclRelTlist = NIL;
OnConflictExpr *result;
+ /*
+ * Bail out if target relation is partitioned table and on conflict
+ * action is UPDATE; there won't be any arbiter indexes to infer the
+ * conflict from. That's because we do not yet support creating
+ * indexes or index constraints on partitioned tables.
+ */
+ if (onConflictClause->action == ONCONFLICT_UPDATE &&
+ pstate->p_target_rangetblentry->relkind == RELKIND_PARTITIONED_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ON CONFLICT ON UPDATE clause is not supported with partitioned tables")));
+
/* Process the arbiter clause, ON CONFLICT ON (...) */
transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems,
&arbiterWhere, &arbiterConstraint);
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 8d005fddd4..d37f57d571 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: ON CONFLICT ON UPDATE clause is not supported with partitioned tables
+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
