Thank you Simon for the review.
On 2017/11/20 7:33, Simon Riggs wrote:
> On 2 August 2017 at 00:56, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote:
>
>> The patch's job is simple:
>
> Patch no longer applies and has some strange behaviours.
>
>> - Remove the check in the parser that causes an error the moment the
>> ON CONFLICT clause is found.
>
> Where is the check and test that blocks ON CONFLICT UPDATE?
That check is in transformInsertStmt() and following is the diff from the
patch that removes it:
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 757a4a8fd1..d680d2285c 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")));
-
ON CONFLICT UPDATE will result in an error because it requires specifying
a conflict_target. Specifying conflict_target will always result in an
error as things stand today, because planner looks for a constraint/index
covering the same and partitioned tables cannot have one.
> My understanding was the patch was intended to only remove the error
> in case of DO NOTHING.
To be precise, the patch is intended to remove the error for the cases
which don't require specifying conflict_target. In INSERT's
documentation, conflict_target is described as follows:
"Specifies which conflicts ON CONFLICT takes the alternative action on by
choosing arbiter indexes. Either performs unique index inference, or names
a constraint explicitly. For ON CONFLICT DO NOTHING, it is optional to
specify a conflict_target; when omitted, conflicts with all usable
constraints (and unique indexes) are handled. For ON CONFLICT DO UPDATE, a
conflict_target must be provided."
Note the "For ON CONFLICT DO NOTHING, it is optional to specify a
conflict_target". That's the case that this patch intends to prevent
error for, that is, the error won't occur if no conflict_target is specified.
>> - 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
>
> Why? There is no caller that needs information.
It is to be used if and when ExecInsert() calls
ExecCheckIndexConstraints() in the code path to handle ON CONFLICT DO
NOTHING that we're intending to support in some cases. Note that it will
only check conflicts for the individual leaf partitions using whatever
constraint-enforcing indexes they might have.
Example:
create table foo (a int) partition by list (a);
create table foo123 partition of foo (a unique) for values in (1, 2, 3);
\d foo123
Table "public.foo123"
Column | Type | Collation | Nullable | Default
=-------+---------+-----------+----------+---------
a | integer | | |
Partition of: foo FOR VALUES IN (1, 2, 3)
Indexes:
"foo123_a_key" UNIQUE CONSTRAINT, btree (a)
Without the patch, parser itself will throw an error:
insert into foo values (1)
on conflict do nothing returning tableoid::regclass, *;
ERROR: ON CONFLICT clause is not supported with partitioned tables
After applying the patch:
insert into foo values (1)
on conflict do nothing returning tableoid::regclass, *;
tableoid | a
=---------+---
foo123 | 1
(1 row)
INSERT 0 1
insert into foo values (1) on conflict do nothing returning
tableoid::regclass, *;
tableoid | a
=---------+---
(0 rows)
INSERT 0 0
That worked because no explicit conflict target was specified. If it is
specified, planner (plancat.c: infer_arbiter_indexes()) will throw an
error, because it cannot find a constraint on foo (which is a partitioned
table).
insert into foo values (1)
on conflict (a) do nothing returning tableoid::regclass, *;
ERROR: there is no unique or exclusion constraint matching the ON
CONFLICT specification
We'll hopefully able to support specifying conflict_target after the
Alvaro's work at [1] is finished.
Meanwhile, rebased patch is attached.
Thanks,
Amit
[1] https://commitfest.postgresql.org/15/1365/
