pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16293: postgres segfaults and returns SQLSTATE 08006
13+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2020-03-23 d35631e37c 44170a5809 Fix potential crash after constraint violation errors in partitioned tables.
2020-03-23 7c83642002 58995db07e 2060999a4d f801ceb696 Add regression tests for constraint errors in partitioned tables.
* BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-10 21:13 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 13+ messages in thread
From: PG Bug reporting form @ 2020-03-10 21:13 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: dwilches@gmail.com
The following bug has been logged on the website:
Bug reference: 16293
Logged by: Daniel WM
Email address: dwilches@gmail.com
PostgreSQL version: 11.7
Operating system: Ubuntu 16.04
Description:
I was getting this error each time I ran a series of autoamted tests in a CI
server:
```
2020-03-06 23:32:57,051 WARN main c.z.h.p.ProxyConnection - HikariPool-2 -
Connection org.postgresql.jdbc.PgConnection@42e3ede4 marked as broken
because of SQLSTATE(08006), ErrorCode(0) {}
org.postgresql.util.PSQLException: An I/O error occurred while sending to
the backend.
at
org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:335)
at
org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:441)
at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:365)
at
org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:143)
at
org.postgresql.jdbc.PgPreparedStatement.executeQuery(PgPreparedStatement.java:106)
```
I looked at `dmesg` and I saw this:
[1383242.997083] postgres[7998]: segfault at 100000048 ip
000055c587913e4b sp 00007fffa492e6f0 error 4 in
postgres[55c587424000+72d000]
Using `gdb` I got this backtrace:
```
Core was generated by `postgres: REDACTED REDACTED 127.0.0.1(49990) INSERT
'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055c587913e4b in pfree ()
(gdb) bt
#0 0x000055c587913e4b in pfree ()
#1 0x000055c587687475 in ExecSetSlotDescriptor ()
#2 0x000055c58767eb61 in ExecConstraints ()
#3 0x000055c5876a0efc in ?? ()
#4 0x000055c5876a2085 in ?? ()
#5 0x000055c58767cd1b in standard_ExecutorRun ()
#6 0x000055c5877d22e5 in ?? ()
#7 0x000055c5877d2538 in ?? ()
#8 0x000055c5877d2855 in ?? ()
#9 0x000055c5877d3427 in PortalRun ()
#10 0x000055c5877cfeec in PostgresMain ()
#11 0x000055c5874ddd37 in ?? ()
#12 0x000055c58775a882 in PostmasterMain ()
#13 0x000055c5874df0e5 in main ()
```
My full version of Postgres is: `PostgreSQL 11.7 (Ubuntu 11.7-1.pgdg16.04+1)
on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 5.4.0-6ubuntu1~16.04.12)
5.4.0 20160609, 64-bit`
I finally found the issue was that one of the tests was trying to insert a
row on a partitioned table, but there was no partition where to put that
row, so it had to go to the default partition. But, the default partition
had a constraint that disallowed that row, so the row couldn't be inserted
and Postgres ended with a segfault.
I enabled query logging and managed to find the offending "insert":
insert into "myschema"."mytable" ("custcode", "custcar", "custdob",
"closed") values ('a33113f2-930c-47de-95a6-b9e07650468a', 'hellow world',
'2020-02-02 01:00:00+00:00', 'f')
That is a partitioned table on the "custdob" column, with these
partitions:
```
\d+ mytable
Table
"myschema.mytable"
Column | Type | Collation | Nullable |
Default | Storage | Stats target | Description
------------+--------------------------+-----------+----------+----------------------------------------+----------+--------------+-------------
id | bigint | | not null |
nextval('mytable_id_seq'::regclass) | plain | |
custcode | uuid | | not null |
| plain | |
custcar | character varying | | not null |
| extended | |
custdob | timestamp with time zone | | not null |
| plain | |
closed | boolean | | not null | false
| plain | |
Partition key: RANGE (custdob)
Partitions: mytable_201902_partition FOR VALUES FROM ('2019-02-01
00:00:00+00') TO ('2019-03-01 00:00:00+00'),
mytable_201903_partition FOR VALUES FROM ('2019-03-01
00:00:00+00') TO ('2019-04-01 00:00:00+00'),
mytable_201908_partition FOR VALUES FROM ('2019-08-02
00:00:00+00') TO ('2019-09-01 00:00:00+00'),
mytable_202003_partition FOR VALUES FROM ('2020-03-01
00:00:00+00') TO ('2020-04-01 00:00:00+00'),
mytable_202004_partition FOR VALUES FROM ('2020-04-01
00:00:00+00') TO ('2020-05-01 00:00:00+00'),
mytable_000000_partition DEFAULT
```
Notice the INSERT wants to insert in February's partition, but that
partition is missing in my CI server, so it should insert the row in the
DEFAULT partition. The issue is, the DEFAULT partition has this
constraint:
```
"mytable_partition_check" CHECK (custdob < '2019-08-02
00:00:00+00'::timestamp with time zone)
```
So Postgres seems to be getting into a bug because it can't insert a record
for February while that constraint is in there. If I drop this constraint
and re-issue the offending INSERT, it works this time.
I first reported this issue in SO:
https://stackoverflow.com/questions/60573071/postgres-segfault-and-returns-sqlstate-08006
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-10 22:19 Tom Lane <tgl@sss.pgh.pa.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2020-03-10 22:19 UTC (permalink / raw)
To: dwilches@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org
PG Bug reporting form <noreply@postgresql.org> writes:
> I finally found the issue was that one of the tests was trying to insert a
> row on a partitioned table, but there was no partition where to put that
> row, so it had to go to the default partition. But, the default partition
> had a constraint that disallowed that row, so the row couldn't be inserted
> and Postgres ended with a segfault.
Hm, works for me:
regression=# create table tt (f1 int, f2 text) partition by list(f1);
CREATE TABLE
regression=# create table t1 partition of tt for values in (1);
CREATE TABLE
regression=# create table tother partition of tt default;
CREATE TABLE
regression=# alter table tother add check (length(f2) > 1);
ALTER TABLE
regression=# insert into tt values (1, 'f');
INSERT 0 1
regression=# insert into tt values (3, 'f');
ERROR: new row for relation "tother" violates check constraint "tother_f2_check"
DETAIL: Failing row contains (3, f).
regression=#
Admittedly this is v11 branch tip not exactly 11.7, but I don't see
anything related-looking in the commit log. So I think there is some
contributing factor you didn't mention. Could you provide a
self-contained reproduction script?
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-10 23:04 Daniel WM <dwilches@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 13+ messages in thread
From: Daniel WM @ 2020-03-10 23:04 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
I tried your steps and it didn't crash here either. I'll try to find a
reproducible set of steps.
I isolated the test that was failing and it didn't fail by itself, so there
is some interaction with my other tests. I tried running all tests in my
laptop and yeap, it segfaults here too. So we can add "Ubuntu 18.04.4" with
this Postgres:
```
# select version();
version
-----------------------------------------------------------------------------------------------------------------------------------
PostgreSQL 11.7 (Ubuntu 11.7-2.pgdg18.04+1) on x86_64-pc-linux-gnu,
compiled by gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0, 64-bit
```
--
Daniel Wilches
On Tue, Mar 10, 2020 at 3:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > I finally found the issue was that one of the tests was trying to insert
> a
> > row on a partitioned table, but there was no partition where to put that
> > row, so it had to go to the default partition. But, the default partition
> > had a constraint that disallowed that row, so the row couldn't be
> inserted
> > and Postgres ended with a segfault.
>
> Hm, works for me:
>
> regression=# create table tt (f1 int, f2 text) partition by list(f1);
> CREATE TABLE
> regression=# create table t1 partition of tt for values in (1);
> CREATE TABLE
> regression=# create table tother partition of tt default;
> CREATE TABLE
> regression=# alter table tother add check (length(f2) > 1);
> ALTER TABLE
> regression=# insert into tt values (1, 'f');
> INSERT 0 1
> regression=# insert into tt values (3, 'f');
> ERROR: new row for relation "tother" violates check constraint
> "tother_f2_check"
> DETAIL: Failing row contains (3, f).
> regression=#
>
> Admittedly this is v11 branch tip not exactly 11.7, but I don't see
> anything related-looking in the commit log. So I think there is some
> contributing factor you didn't mention. Could you provide a
> self-contained reproduction script?
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-14 20:43 Daniel WM <dwilches@gmail.com>
parent: Daniel WM <dwilches@gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Daniel WM @ 2020-03-14 20:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs@lists.postgresql.org
Hello,
I have finally isolated the issue and I have a set of steps that reliably
cause the segfault:
CREATE TABLE parent_table (
custdob timestamp with time zone not null,
closed BOOLEAN NOT NULL DEFAULT FALSE
) PARTITION BY RANGE (custdob);
CREATE TABLE default_partition (
custdob timestamp with time zone not null,
CONSTRAINT dummy_check CHECK (custdob < '2019-08-02T00:00Z')
);
-- This is needed for the crash, if I add this column when creating the
table "default_partition" then I don't get the crash, but when it's added
with an "ALTER TABLE" then I get the crash.
ALTER TABLE default_partition ADD COLUMN closed BOOLEAN NOT NULL DEFAULT
FALSE;
ALTER TABLE parent_table ATTACH PARTITION default_partition DEFAULT;
INSERT INTO parent_table VALUES ('2020-02-02 01:00:00+00:00', 'f');
--
Daniel Wilches
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-14 21:21 Tom Lane <tgl@sss.pgh.pa.us>
parent: Daniel WM <dwilches@gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Tom Lane @ 2020-03-14 21:21 UTC (permalink / raw)
To: Daniel WM <dwilches@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org, Andres Freund <andres@anarazel.de>
Daniel WM <dwilches@gmail.com> writes:
> I have finally isolated the issue and I have a set of steps that reliably
> cause the segfault:
Interesting. I do *not* see a crash in v12 or HEAD, but v11 gives
me an assertion failure:
TRAP: FailedAssertion("!(!slot->tts_fixedTupleDescriptor)", File: "execTuples.c", Line: 284)
2020-03-14 17:16:06.626 EDT [1101] LOG: server process (PID 20822) was terminated by signal 6: Aborted
2020-03-14 17:16:06.626 EDT [1101] DETAIL: Failed process was running: INSERT INTO parent_table VALUES ('2020-02-02 01:00:00+00:00', 'f');
here:
#3 0x000000000063ef45 in ExecSetSlotDescriptor (slot=0x1cf4060,
tupdesc=0x7f903a2dd0f8) at execTuples.c:284
#4 0x00000000006360eb in ExecConstraints (resultRelInfo=0x1cf47b8,
slot=0x1cf4060, estate=0x1cf3778) at execMain.c:2062
#5 0x000000000065a44d in ExecInsert (mtstate=0x1cf3ae0, slot=0x1cf4060,
planSlot=0x1cf4060, estate=0x1cf3778, canSetTag=true)
at nodeModifyTable.c:398
#6 0x000000000065b80b in ExecModifyTable (pstate=<value optimized out>)
at nodeModifyTable.c:2159
#7 0x0000000000636c27 in ExecProcNode (queryDesc=0x1ce51e8,
direction=<value optimized out>, count=0, execute_once=224)
at ../../../src/include/executor/executor.h:247
#8 ExecutePlan (queryDesc=0x1ce51e8, direction=<value optimized out>,
count=0, execute_once=224) at execMain.c:1723
#9 standard_ExecutorRun (queryDesc=0x1ce51e8,
direction=<value optimized out>, count=0, execute_once=224)
So this looks like something to do with Andres' tupletableslot work.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-14 21:26 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 13+ messages in thread
From: Andres Freund @ 2020-03-14 21:26 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Daniel WM <dwilches@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi,
On 2020-03-14 17:21:04 -0400, Tom Lane wrote:
> Daniel WM <dwilches@gmail.com> writes:
> > I have finally isolated the issue and I have a set of steps that reliably
> > cause the segfault:
>
> Interesting. I do *not* see a crash in v12 or HEAD, but v11 gives
> me an assertion failure:
>
> TRAP: FailedAssertion("!(!slot->tts_fixedTupleDescriptor)", File: "execTuples.c", Line: 284)
> 2020-03-14 17:16:06.626 EDT [1101] LOG: server process (PID 20822) was terminated by signal 6: Aborted
> 2020-03-14 17:16:06.626 EDT [1101] DETAIL: Failed process was running: INSERT INTO parent_table VALUES ('2020-02-02 01:00:00+00:00', 'f');
>
> here:
>
> #3 0x000000000063ef45 in ExecSetSlotDescriptor (slot=0x1cf4060,
> tupdesc=0x7f903a2dd0f8) at execTuples.c:284
> #4 0x00000000006360eb in ExecConstraints (resultRelInfo=0x1cf47b8,
> slot=0x1cf4060, estate=0x1cf3778) at execMain.c:2062
> #5 0x000000000065a44d in ExecInsert (mtstate=0x1cf3ae0, slot=0x1cf4060,
> planSlot=0x1cf4060, estate=0x1cf3778, canSetTag=true)
> at nodeModifyTable.c:398
> #6 0x000000000065b80b in ExecModifyTable (pstate=<value optimized out>)
> at nodeModifyTable.c:2159
> #7 0x0000000000636c27 in ExecProcNode (queryDesc=0x1ce51e8,
> direction=<value optimized out>, count=0, execute_once=224)
> at ../../../src/include/executor/executor.h:247
> #8 ExecutePlan (queryDesc=0x1ce51e8, direction=<value optimized out>,
> count=0, execute_once=224) at execMain.c:1723
> #9 standard_ExecutorRun (queryDesc=0x1ce51e8,
> direction=<value optimized out>, count=0, execute_once=224)
>
> So this looks like something to do with Andres' tupletableslot work.
Interesting. There wasn't that much in v11. It's not the general rework,
but just the allocation of the descriptor together slot when the
descriptor is known at creation time - or a conflict with concurrent
partitioning work. It's plausible that enough changed in master for
that to not be a problem anymore.
Looking.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-15 00:35 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 13+ messages in thread
From: Andres Freund @ 2020-03-15 00:35 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Daniel WM <dwilches@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi,
On 2020-03-14 14:26:57 -0700, Andres Freund wrote:
> On 2020-03-14 17:21:04 -0400, Tom Lane wrote:
> > Interesting. I do *not* see a crash in v12 or HEAD, but v11 gives
> > me an assertion failure:
> >
> > TRAP: FailedAssertion("!(!slot->tts_fixedTupleDescriptor)", File: "execTuples.c", Line: 284)
> > 2020-03-14 17:16:06.626 EDT [1101] LOG: server process (PID 20822) was terminated by signal 6: Aborted
> > 2020-03-14 17:16:06.626 EDT [1101] DETAIL: Failed process was running: INSERT INTO parent_table VALUES ('2020-02-02 01:00:00+00:00', 'f');
> >
> > here:
> >
> > #3 0x000000000063ef45 in ExecSetSlotDescriptor (slot=0x1cf4060,
> > tupdesc=0x7f903a2dd0f8) at execTuples.c:284
> > #4 0x00000000006360eb in ExecConstraints (resultRelInfo=0x1cf47b8,
> > slot=0x1cf4060, estate=0x1cf3778) at execMain.c:2062
> > #5 0x000000000065a44d in ExecInsert (mtstate=0x1cf3ae0, slot=0x1cf4060,
> > planSlot=0x1cf4060, estate=0x1cf3778, canSetTag=true)
> > at nodeModifyTable.c:398
> > #6 0x000000000065b80b in ExecModifyTable (pstate=<value optimized out>)
> > at nodeModifyTable.c:2159
> > #7 0x0000000000636c27 in ExecProcNode (queryDesc=0x1ce51e8,
> > direction=<value optimized out>, count=0, execute_once=224)
> > at ../../../src/include/executor/executor.h:247
> > #8 ExecutePlan (queryDesc=0x1ce51e8, direction=<value optimized out>,
> > count=0, execute_once=224) at execMain.c:1723
> > #9 standard_ExecutorRun (queryDesc=0x1ce51e8,
> > direction=<value optimized out>, count=0, execute_once=224)
> >
> > So this looks like something to do with Andres' tupletableslot work.
>
> Interesting. There wasn't that much in v11. It's not the general rework,
> but just the allocation of the descriptor together slot when the
> descriptor is known at creation time - or a conflict with concurrent
> partitioning work. It's plausible that enough changed in master for
> that to not be a problem anymore.
Hm. So the relevant slot is created at (huray for rr making it trivial
to determine that):
#0 0x000055d6c1b127c6 in MakeTupleTableSlot (tupleDesc=0x55d6c3dd64f8) at /home/andres/src/postgresql-11/src/backend/executor/execTuples.c:135
#1 0x000055d6c1b12895 in ExecAllocTableSlot (tupleTable=0x55d6c3dd5dd8, desc=0x55d6c3dd64f8)
at /home/andres/src/postgresql-11/src/backend/executor/execTuples.c:169
#2 0x000055d6c1b1389a in ExecInitResultTupleSlotTL (estate=0x55d6c3dd5d28, planstate=0x55d6c3dd62e8)
at /home/andres/src/postgresql-11/src/backend/executor/execTuples.c:907
#3 0x000055d6c1b3e98b in ExecInitResult (node=0x55d6c3de2638, estate=0x55d6c3dd5d28, eflags=0)
at /home/andres/src/postgresql-11/src/backend/executor/nodeResult.c:220
#4 0x000055d6c1b0e690 in ExecInitNode (node=0x55d6c3de2638, estate=0x55d6c3dd5d28, eflags=0)
at /home/andres/src/postgresql-11/src/backend/executor/execProcnode.c:164
#5 0x000055d6c1b3beec in ExecInitModifyTable (node=0x55d6c3d0cdb0, estate=0x55d6c3dd5d28, eflags=0)
at /home/andres/src/postgresql-11/src/backend/executor/nodeModifyTable.c:2304
#6 0x000055d6c1b0e6ce in ExecInitNode (node=0x55d6c3d0cdb0, estate=0x55d6c3dd5d28, eflags=0)
at /home/andres/src/postgresql-11/src/backend/executor/execProcnode.c:174
#7 0x000055d6c1b04c21 in InitPlan (queryDesc=0x55d6c3dec888, eflags=0) at /home/andres/src/postgresql-11/src/backend/executor/execMain.c:1046
I don't think it's ok for ExecConstraint() to overwrite the tuple
descriptor of a slot "owned" by nodeResult.c. Am I missing something, or
is that broken?
In v12+ that problem doesn't exist, because we simply allocate a new
slot (this is just for printing an error message).
There's other places using ExecSetSlotDescriptor(), e.g.
else if (resultRelInfo->ri_FdwRoutine)
{
HeapTuple tuple;
/*
* delete from foreign table: let the FDW do it
*
* We offer the trigger tuple slot as a place to store RETURNING data,
* although the FDW can return some other slot if it wants. Set up
* the slot's tupdesc so the FDW doesn't need to do that for itself.
*/
slot = estate->es_trig_tuple_slot;
if (slot->tts_tupleDescriptor != RelationGetDescr(resultRelationDesc))
ExecSetSlotDescriptor(slot, RelationGetDescr(resultRelationDesc));
slot = resultRelInfo->ri_FdwRoutine->ExecForeignDelete(estate,
resultRelInfo,
slot,
planSlot);
but that's different, because it's using a slot (kind of) owned by the
modify node, rather just a random subsidiary node's slot.
I think the only reason this issue doesn't have actually bad
consequences is that we throw an error afterwards - which'll result in
the executor tree not being used any further.
I'm inclined to simply copy v12's approach of just ad-hoc creating a
slot in those routines?
It seems fairly insane how many places have this approximate code,
btw. In 11 we have a copy of nearly the same ~40 lines each in at least
ExecPartitionCheckEmitError(), ExecConstraints (twice) and
ExecWitchCheckOption().
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-17 10:49 Amit Langote <amitlangote09@gmail.com>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 13+ messages in thread
From: Amit Langote @ 2020-03-17 10:49 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Daniel WM <dwilches@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi,
On Sun, Mar 15, 2020 at 9:36 AM Andres Freund <andres@anarazel.de> wrote:
> On 2020-03-14 14:26:57 -0700, Andres Freund wrote:
> > On 2020-03-14 17:21:04 -0400, Tom Lane wrote:
> > > Interesting. I do *not* see a crash in v12 or HEAD, but v11 gives
> > > me an assertion failure:
> > >
> > > TRAP: FailedAssertion("!(!slot->tts_fixedTupleDescriptor)", File: "execTuples.c", Line: 284)
> > > 2020-03-14 17:16:06.626 EDT [1101] LOG: server process (PID 20822) was terminated by signal 6: Aborted
> > > 2020-03-14 17:16:06.626 EDT [1101] DETAIL: Failed process was running: INSERT INTO parent_table VALUES ('2020-02-02 01:00:00+00:00', 'f');
> > >
> > > here:
> > >
> > > #3 0x000000000063ef45 in ExecSetSlotDescriptor (slot=0x1cf4060,
> > > tupdesc=0x7f903a2dd0f8) at execTuples.c:284
> > > #4 0x00000000006360eb in ExecConstraints (resultRelInfo=0x1cf47b8,
> > > slot=0x1cf4060, estate=0x1cf3778) at execMain.c:2062
> > > #5 0x000000000065a44d in ExecInsert (mtstate=0x1cf3ae0, slot=0x1cf4060,
> > > planSlot=0x1cf4060, estate=0x1cf3778, canSetTag=true)
> > > at nodeModifyTable.c:398
> > > #6 0x000000000065b80b in ExecModifyTable (pstate=<value optimized out>)
> > > at nodeModifyTable.c:2159
> > > #7 0x0000000000636c27 in ExecProcNode (queryDesc=0x1ce51e8,
> > > direction=<value optimized out>, count=0, execute_once=224)
> > > at ../../../src/include/executor/executor.h:247
> > > #8 ExecutePlan (queryDesc=0x1ce51e8, direction=<value optimized out>,
> > > count=0, execute_once=224) at execMain.c:1723
> > > #9 standard_ExecutorRun (queryDesc=0x1ce51e8,
> > > direction=<value optimized out>, count=0, execute_once=224)
> > >
> > > So this looks like something to do with Andres' tupletableslot work.
> >
> > Interesting. There wasn't that much in v11. It's not the general rework,
> > but just the allocation of the descriptor together slot when the
> > descriptor is known at creation time - or a conflict with concurrent
> > partitioning work. It's plausible that enough changed in master for
> > that to not be a problem anymore.
>
> Hm. So the relevant slot is created at (huray for rr making it trivial
> to determine that):
>
> #0 0x000055d6c1b127c6 in MakeTupleTableSlot (tupleDesc=0x55d6c3dd64f8) at /home/andres/src/postgresql-11/src/backend/executor/execTuples.c:135
> #1 0x000055d6c1b12895 in ExecAllocTableSlot (tupleTable=0x55d6c3dd5dd8, desc=0x55d6c3dd64f8)
> at /home/andres/src/postgresql-11/src/backend/executor/execTuples.c:169
> #2 0x000055d6c1b1389a in ExecInitResultTupleSlotTL (estate=0x55d6c3dd5d28, planstate=0x55d6c3dd62e8)
> at /home/andres/src/postgresql-11/src/backend/executor/execTuples.c:907
> #3 0x000055d6c1b3e98b in ExecInitResult (node=0x55d6c3de2638, estate=0x55d6c3dd5d28, eflags=0)
> at /home/andres/src/postgresql-11/src/backend/executor/nodeResult.c:220
> #4 0x000055d6c1b0e690 in ExecInitNode (node=0x55d6c3de2638, estate=0x55d6c3dd5d28, eflags=0)
> at /home/andres/src/postgresql-11/src/backend/executor/execProcnode.c:164
> #5 0x000055d6c1b3beec in ExecInitModifyTable (node=0x55d6c3d0cdb0, estate=0x55d6c3dd5d28, eflags=0)
> at /home/andres/src/postgresql-11/src/backend/executor/nodeModifyTable.c:2304
> #6 0x000055d6c1b0e6ce in ExecInitNode (node=0x55d6c3d0cdb0, estate=0x55d6c3dd5d28, eflags=0)
> at /home/andres/src/postgresql-11/src/backend/executor/execProcnode.c:174
> #7 0x000055d6c1b04c21 in InitPlan (queryDesc=0x55d6c3dec888, eflags=0) at /home/andres/src/postgresql-11/src/backend/executor/execMain.c:1046
>
> I don't think it's ok for ExecConstraint() to overwrite the tuple
> descriptor of a slot "owned" by nodeResult.c. Am I missing something, or
> is that broken?
Looking into it, that partitioning code in ExecConstraint() would not
"normally" overwrite a slot that is not managed by partitioning.
Normally, there would be a dedicated "partition" slot that would be
used, but only if tuple conversion from root parent to leaf partition
is necessary before routing the tuple.
What is not "normal" in this case is that tuple conversion is deemed
necessary when converting from leaf partition to root parent whereas
not in the other direction. It's because one of the partition's
attributes has atthasmissing set to true, which triggers this code in
convert_tuples_by_name():
/*
* If the input column has a missing attribute, we need a
* conversion.
*/
if (inatt->atthasmissing)
{
same = false;
break;
}
This asymmetry causes the code in ExecConstraints() to try to convert
the tuple whereas no conversion would have been performed before,
meaning to dedicated partition slot would have been passed to
ExecConstraints().
> In v12+ that problem doesn't exist, because we simply allocate a new
> slot (this is just for printing an error message).
Right.
> I'm inclined to simply copy v12's approach of just ad-hoc creating a
> slot in those routines?
Agreed.
> It seems fairly insane how many places have this approximate code,
> btw. In 11 we have a copy of nearly the same ~40 lines each in at least
> ExecPartitionCheckEmitError(), ExecConstraints (twice) and
> ExecWitchCheckOption().
That is certainly bad. I think we should get that code in one place
and inside ExecBuildSlotValueDescription() seems like a good place,
because that's what needs a converted tuple to build a string from it.
I have tried that in the attached -- need different patches for
different branches as there have been a lot of small changes in this
code over releases.
--
Thank you,
Amit
Attachments: download all (4 files, .tar.gz)
ExecBuildSlotValueDescription-partitions-v11.patch (application/octet-stream, 10.7K) download
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 66cac46acf..5dbba3b6a0 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -94,10 +94,9 @@ static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
-static char *ExecBuildSlotValueDescription(Oid reloid,
+static char *ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen);
static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate,
Plan *planTree);
@@ -1906,49 +1905,14 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- Relation orig_rel = rel;
- TupleDesc tupdesc = RelationGetDescr(rel);
char *val_desc;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
- /*
- * Need to first convert the tuple to the root partitioned table's row
- * type. For details, check similar comments in ExecConstraints().
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
- RelationGetRelationName(orig_rel)),
+ RelationGetRelationName(resultRelInfo->ri_RelationDesc)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
}
@@ -1970,9 +1934,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
Assert(constr || resultRelInfo->ri_PartitionCheck);
@@ -1988,49 +1949,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if (att->attnotnull && slot_attisnull(slot, attrChk))
{
char *val_desc;
- Relation orig_rel = rel;
- TupleDesc orig_tupdesc = RelationGetDescr(rel);
- /*
- * If the tuple has been routed, it's been converted to the
- * partition's rowtype, which might differ from the root
- * table's. We must convert it back to the root table's
- * rowtype so that val_desc shown error message matches the
- * input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(orig_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
-
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("null value in column \"%s\" violates not-null constraint",
NameStr(att->attname)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtablecol(orig_rel, attrChk)));
+ errtablecol(rel, attrChk)));
}
}
}
@@ -2042,42 +1971,15 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
{
char *val_desc;
- Relation orig_rel = rel;
- /* See the comment above. */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
- RelationGetRelationName(orig_rel), failed),
+ RelationGetRelationName(rel), failed),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtableconstraint(orig_rel, failed)));
+ errtableconstraint(rel, failed)));
}
}
}
@@ -2095,8 +1997,6 @@ void
ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- TupleDesc tupdesc = RelationGetDescr(rel);
ExprContext *econtext;
ListCell *l1,
*l2;
@@ -2134,9 +2034,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
if (!ExecQual(wcoExpr, econtext))
{
char *val_desc;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
switch (wco->kind)
{
@@ -2150,33 +2047,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* USING policy.
*/
case WCO_VIEW_CHECK:
- /* See the comment in ExecConstraints(). */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
@@ -2229,8 +2102,7 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
*
* Also, unlike the case with index entries, we need to be prepared to ignore
* dropped columns. We used to use the slot's tuple descriptor to decode the
- * data, but the slot's descriptor doesn't identify dropped columns, so we
- * now need to be passed the relation's descriptor.
+ * data, but the slot's descriptor doesn't identify dropped columns.
*
* Note that, like BuildIndexValueDescription, if the user does not have
* permission to view any of the columns involved, a NULL is returned. Unlike
@@ -2239,10 +2111,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* columns they are.
*/
static char *
-ExecBuildSlotValueDescription(Oid reloid,
+ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen)
{
StringInfoData buf;
@@ -2253,6 +2124,9 @@ ExecBuildSlotValueDescription(Oid reloid,
AclResult aclresult;
bool table_perm = false;
bool any_perm = false;
+ Oid reloid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ TupleDesc tupdesc;
+ Bitmapset *modifiedCols;
/*
* Check if RLS is enabled and should be active for the relation; if so,
@@ -2262,6 +2136,39 @@ ExecBuildSlotValueDescription(Oid reloid,
if (check_enable_rls(reloid, InvalidOid, true) == RLS_ENABLED)
return NULL;
+ /*
+ * If the tuple has been routed, it's been converted to the partition's
+ * rowtype, which might differ from the root table's. We must convert it
+ * back to the root table's rowtype so that val_desc matches the input
+ * tuple.
+ */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ HeapTuple tuple = ExecFetchSlotTuple(slot);
+ TupleDesc old_tupdesc;
+ TupleConversionMap *map;
+
+ tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ /* a reverse map */
+ map = convert_tuples_by_name(old_tupdesc, tupdesc,
+ gettext_noop("could not convert row type"));
+ if (map != NULL)
+ {
+ tuple = do_convert_tuple(tuple, map);
+
+ /* Create a temporary slot with the new tuple descriptor. */
+ slot = MakeTupleTableSlot(tupdesc);
+ ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ }
+ }
+ else
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+
+ modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
+ GetUpdatedColumns(resultRelInfo, estate));
+
initStringInfo(&buf);
appendStringInfoChar(&buf, '(');
ExecBuildSlotValueDescription-partitions-v10.patch (application/octet-stream, 10.8K) download
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6c6d098e54..9e54d27ac2 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -94,10 +94,9 @@ static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
-static char *ExecBuildSlotValueDescription(Oid reloid,
+static char *ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen);
static char *ExecBuildSlotPartitionKeyDescription(Relation rel,
Datum *values,
@@ -1866,10 +1865,6 @@ ExecPartitionCheck(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
EState *estate)
{
Relation rel = resultRelInfo->ri_RelationDesc;
- TupleDesc tupdesc = RelationGetDescr(rel);
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
ExprContext *econtext;
/*
@@ -1900,40 +1895,13 @@ ExecPartitionCheck(ResultRelInfo *resultRelInfo, TupleTableSlot *slot,
if (!ExecCheck(resultRelInfo->ri_PartitionCheckExpr, econtext))
{
char *val_desc;
- Relation orig_rel = rel;
- /* See the comment above. */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
- RelationGetRelationName(orig_rel)),
+ RelationGetRelationName(rel)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
}
}
@@ -1955,9 +1923,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
Assert(constr || resultRelInfo->ri_PartitionCheck);
@@ -1972,49 +1937,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
slot_attisnull(slot, attrChk))
{
char *val_desc;
- Relation orig_rel = rel;
- TupleDesc orig_tupdesc = RelationGetDescr(rel);
- /*
- * If the tuple has been routed, it's been converted to the
- * partition's rowtype, which might differ from the root
- * table's. We must convert it back to the root table's
- * rowtype so that val_desc shown error message matches the
- * input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(orig_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
-
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("null value in column \"%s\" violates not-null constraint",
- NameStr(orig_tupdesc->attrs[attrChk - 1]->attname)),
+ NameStr(tupdesc->attrs[attrChk - 1]->attname)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtablecol(orig_rel, attrChk)));
+ errtablecol(rel, attrChk)));
}
}
}
@@ -2026,42 +1959,15 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
{
char *val_desc;
- Relation orig_rel = rel;
- /* See the comment above. */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
- RelationGetRelationName(orig_rel), failed),
+ RelationGetRelationName(rel), failed),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtableconstraint(orig_rel, failed)));
+ errtableconstraint(rel, failed)));
}
}
@@ -2083,8 +1989,6 @@ void
ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- TupleDesc tupdesc = RelationGetDescr(rel);
ExprContext *econtext;
ListCell *l1,
*l2;
@@ -2122,9 +2026,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
if (!ExecQual(wcoExpr, econtext))
{
char *val_desc;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
switch (wco->kind)
{
@@ -2138,33 +2039,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* USING policy.
*/
case WCO_VIEW_CHECK:
- /* See the comment in ExecConstraints(). */
- if (resultRelInfo->ri_PartitionRoot)
- {
- HeapTuple tuple = ExecFetchSlotTuple(slot);
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- TupleConversionMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
- if (map != NULL)
- {
- tuple = do_convert_tuple(tuple, map);
- ExecSetSlotDescriptor(slot, tupdesc);
- ExecStoreTuple(tuple, slot, InvalidBuffer, false);
- }
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
@@ -2217,8 +2094,7 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
*
* Also, unlike the case with index entries, we need to be prepared to ignore
* dropped columns. We used to use the slot's tuple descriptor to decode the
- * data, but the slot's descriptor doesn't identify dropped columns, so we
- * now need to be passed the relation's descriptor.
+ * data, but the slot's descriptor doesn't identify dropped columns.
*
* Note that, like BuildIndexValueDescription, if the user does not have
* permission to view any of the columns involved, a NULL is returned. Unlike
@@ -2227,10 +2103,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* columns they are.
*/
static char *
-ExecBuildSlotValueDescription(Oid reloid,
+ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen)
{
StringInfoData buf;
@@ -2241,6 +2116,9 @@ ExecBuildSlotValueDescription(Oid reloid,
AclResult aclresult;
bool table_perm = false;
bool any_perm = false;
+ Oid reloid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ TupleDesc tupdesc;
+ Bitmapset *modifiedCols;
/*
* Check if RLS is enabled and should be active for the relation; if so,
@@ -2250,6 +2128,39 @@ ExecBuildSlotValueDescription(Oid reloid,
if (check_enable_rls(reloid, InvalidOid, true) == RLS_ENABLED)
return NULL;
+ /*
+ * If the tuple has been routed, it's been converted to the partition's
+ * rowtype, which might differ from the root table's. We must convert it
+ * back to the root table's rowtype so that val_desc matches the input
+ * tuple.
+ */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ HeapTuple tuple = ExecFetchSlotTuple(slot);
+ TupleDesc old_tupdesc;
+ TupleConversionMap *map;
+
+ tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ /* a reverse map */
+ map = convert_tuples_by_name(old_tupdesc, tupdesc,
+ gettext_noop("could not convert row type"));
+ if (map != NULL)
+ {
+ tuple = do_convert_tuple(tuple, map);
+
+ /* Create a temporary slot with the new tuple descriptor. */
+ slot = MakeSingleTupleTableSlot(tupdesc);
+ ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ }
+ }
+ else
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+
+ modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
+ GetUpdatedColumns(resultRelInfo, estate));
+
initStringInfo(&buf);
appendStringInfoChar(&buf, '(');
ExecBuildSlotValueDescription-partitions-HEAD.patch (application/octet-stream, 10.5K) download
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 28130fbc2b..cd516ab2d3 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -93,11 +93,10 @@ static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
-static char *ExecBuildSlotValueDescription(Oid reloid,
- TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
- int maxfieldlen);
+static char *ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot,
+ int maxfieldlen);
static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
/*
@@ -1829,51 +1828,10 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- Oid root_relid;
- TupleDesc tupdesc;
char *val_desc;
- Bitmapset *modifiedCols;
- /*
- * If the tuple has been routed, it's been converted to the partition's
- * rowtype, which might differ from the root table's. We must convert it
- * back to the root table's rowtype so that val_desc in the error message
- * matches the input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc;
- AttrMap *map;
-
- root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
- tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
-
- old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(old_tupdesc, tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so allocate a
- * new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
- else
- {
- root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
- tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
- }
-
- modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
- GetUpdatedColumns(resultRelInfo, estate));
-
- val_desc = ExecBuildSlotValueDescription(root_relid,
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
@@ -1899,9 +1857,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
Assert(constr || resultRelInfo->ri_PartitionCheck);
@@ -1917,51 +1872,19 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if (att->attnotnull && slot_attisnull(slot, attrChk))
{
char *val_desc;
- Relation orig_rel = rel;
- TupleDesc orig_tupdesc = RelationGetDescr(rel);
- /*
- * If the tuple has been routed, it's been converted to the
- * partition's rowtype, which might differ from the root
- * table's. We must convert it back to the root table's
- * rowtype so that val_desc shown error message matches the
- * input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- AttrMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(orig_tupdesc,
- tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so
- * allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("null value in column \"%s\" of relation \"%s\" violates not-null constraint",
NameStr(att->attname),
- RelationGetRelationName(orig_rel)),
+ RelationGetRelationName(rel)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtablecol(orig_rel, attrChk)));
+ errtablecol(rel, attrChk)));
}
}
}
@@ -1973,43 +1896,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
{
char *val_desc;
- Relation orig_rel = rel;
- /* See the comment above. */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- AttrMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(old_tupdesc,
- tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so
- * allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
- RelationGetRelationName(orig_rel), failed),
+ RelationGetRelationName(rel), failed),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtableconstraint(orig_rel, failed)));
+ errtableconstraint(rel, failed)));
}
}
}
@@ -2027,8 +1924,6 @@ void
ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- TupleDesc tupdesc = RelationGetDescr(rel);
ExprContext *econtext;
ListCell *l1,
*l2;
@@ -2066,9 +1961,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
if (!ExecQual(wcoExpr, econtext))
{
char *val_desc;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
switch (wco->kind)
{
@@ -2082,34 +1974,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* USING policy.
*/
case WCO_VIEW_CHECK:
- /* See the comment in ExecConstraints(). */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- AttrMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(old_tupdesc,
- tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed,
- * so allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
@@ -2162,8 +2029,7 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
*
* Also, unlike the case with index entries, we need to be prepared to ignore
* dropped columns. We used to use the slot's tuple descriptor to decode the
- * data, but the slot's descriptor doesn't identify dropped columns, so we
- * now need to be passed the relation's descriptor.
+ * data, but the slot's descriptor doesn't identify dropped columns.
*
* Note that, like BuildIndexValueDescription, if the user does not have
* permission to view any of the columns involved, a NULL is returned. Unlike
@@ -2172,10 +2038,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* columns they are.
*/
static char *
-ExecBuildSlotValueDescription(Oid reloid,
+ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen)
{
StringInfoData buf;
@@ -2186,6 +2051,9 @@ ExecBuildSlotValueDescription(Oid reloid,
AclResult aclresult;
bool table_perm = false;
bool any_perm = false;
+ Oid reloid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ TupleDesc tupdesc;
+ Bitmapset *modifiedCols;
/*
* Check if RLS is enabled and should be active for the relation; if so,
@@ -2195,6 +2063,34 @@ ExecBuildSlotValueDescription(Oid reloid,
if (check_enable_rls(reloid, InvalidOid, true) == RLS_ENABLED)
return NULL;
+ /*
+ * If the tuple has been routed, it's been converted to the partition's
+ * rowtype, which might differ from the root table's. We must convert it
+ * back to the root table's rowtype so that val_desc matches the input
+ * tuple.
+ */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ TupleDesc old_tupdesc;
+ AttrMap *map;
+
+ tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ /* a reverse map */
+ map = build_attrmap_by_name_if_req(old_tupdesc, tupdesc);
+
+ /* Note that this uses a new slot for the converted tuple. */
+ if (map != NULL)
+ slot = execute_attr_map_slot(map, slot,
+ MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
+ }
+ else
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+
+ modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
+ GetUpdatedColumns(resultRelInfo, estate));
+
initStringInfo(&buf);
appendStringInfoChar(&buf, '(');
ExecBuildSlotValueDescription-partitions-v12.patch (application/octet-stream, 10.9K) download
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 2275a2988f..7ae7a2aacd 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -93,11 +93,10 @@ static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
-static char *ExecBuildSlotValueDescription(Oid reloid,
- TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
- int maxfieldlen);
+static char *ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot,
+ int maxfieldlen);
static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
/*
@@ -1829,52 +1828,10 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- Oid root_relid;
- TupleDesc tupdesc;
char *val_desc;
- Bitmapset *modifiedCols;
- /*
- * If the tuple has been routed, it's been converted to the partition's
- * rowtype, which might differ from the root table's. We must convert it
- * back to the root table's rowtype so that val_desc in the error message
- * matches the input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc;
- AttrNumber *map;
-
- root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
- tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
-
- old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
- /* a reverse map */
- map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc,
- gettext_noop("could not convert row type"));
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so allocate a
- * new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
- else
- {
- root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
- tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
- }
-
- modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
- GetUpdatedColumns(resultRelInfo, estate));
-
- val_desc = ExecBuildSlotValueDescription(root_relid,
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
@@ -1900,9 +1857,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
Assert(constr || resultRelInfo->ri_PartitionCheck);
@@ -1918,51 +1872,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if (att->attnotnull && slot_attisnull(slot, attrChk))
{
char *val_desc;
- Relation orig_rel = rel;
- TupleDesc orig_tupdesc = RelationGetDescr(rel);
- /*
- * If the tuple has been routed, it's been converted to the
- * partition's rowtype, which might differ from the root
- * table's. We must convert it back to the root table's
- * rowtype so that val_desc shown error message matches the
- * input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- AttrNumber *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name_map_if_req(orig_tupdesc,
- tupdesc,
- gettext_noop("could not convert row type"));
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so
- * allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
-
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("null value in column \"%s\" violates not-null constraint",
NameStr(att->attname)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtablecol(orig_rel, attrChk)));
+ errtablecol(rel, attrChk)));
}
}
}
@@ -1974,44 +1894,15 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
{
char *val_desc;
- Relation orig_rel = rel;
- /* See the comment above. */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- AttrNumber *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name_map_if_req(old_tupdesc,
- tupdesc,
- gettext_noop("could not convert row type"));
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so
- * allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
- RelationGetRelationName(orig_rel), failed),
+ RelationGetRelationName(rel), failed),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtableconstraint(orig_rel, failed)));
+ errtableconstraint(rel, failed)));
}
}
}
@@ -2029,8 +1920,6 @@ void
ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- TupleDesc tupdesc = RelationGetDescr(rel);
ExprContext *econtext;
ListCell *l1,
*l2;
@@ -2068,9 +1957,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
if (!ExecQual(wcoExpr, econtext))
{
char *val_desc;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
switch (wco->kind)
{
@@ -2084,37 +1970,10 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* USING policy.
*/
case WCO_VIEW_CHECK:
- /* See the comment in ExecConstraints(). */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- AttrNumber *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = convert_tuples_by_name_map_if_req(old_tupdesc,
- tupdesc,
- gettext_noop("could not convert row type"));
-
- /*
- * Partition-specific slot's tupdesc can't be changed,
- * so allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
-
ereport(ERROR,
(errcode(ERRCODE_WITH_CHECK_OPTION_VIOLATION),
errmsg("new row violates check option for view \"%s\"",
@@ -2165,8 +2024,7 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
*
* Also, unlike the case with index entries, we need to be prepared to ignore
* dropped columns. We used to use the slot's tuple descriptor to decode the
- * data, but the slot's descriptor doesn't identify dropped columns, so we
- * now need to be passed the relation's descriptor.
+ * data, but the slot's descriptor doesn't identify dropped columns.
*
* Note that, like BuildIndexValueDescription, if the user does not have
* permission to view any of the columns involved, a NULL is returned. Unlike
@@ -2175,10 +2033,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* columns they are.
*/
static char *
-ExecBuildSlotValueDescription(Oid reloid,
+ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen)
{
StringInfoData buf;
@@ -2189,6 +2046,9 @@ ExecBuildSlotValueDescription(Oid reloid,
AclResult aclresult;
bool table_perm = false;
bool any_perm = false;
+ Oid reloid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ TupleDesc tupdesc;
+ Bitmapset *modifiedCols;
/*
* Check if RLS is enabled and should be active for the relation; if so,
@@ -2198,6 +2058,35 @@ ExecBuildSlotValueDescription(Oid reloid,
if (check_enable_rls(reloid, InvalidOid, true) == RLS_ENABLED)
return NULL;
+ /*
+ * If the tuple has been routed, it's been converted to the partition's
+ * rowtype, which might differ from the root table's. We must convert it
+ * back to the root table's rowtype so that val_desc matches the input
+ * tuple.
+ */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ TupleDesc old_tupdesc;
+ AttrNumber *map;
+
+ tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ /* a reverse map */
+ map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc,
+ gettext_noop("could not convert row type"));
+
+ /* Note that this uses a new slot for the converted tuple. */
+ if (map != NULL)
+ slot = execute_attr_map_slot(map, slot,
+ MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
+ }
+ else
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+
+ modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
+ GetUpdatedColumns(resultRelInfo, estate));
+
initStringInfo(&buf);
appendStringInfoChar(&buf, '(');
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-22 06:52 Andres Freund <andres@anarazel.de>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 13+ messages in thread
From: Andres Freund @ 2020-03-22 06:52 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Daniel WM <dwilches@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi,
On 2020-03-17 19:49:43 +0900, Amit Langote wrote:
> On Sun, Mar 15, 2020 at 9:36 AM Andres Freund <andres@anarazel.de> wrote:
> > I don't think it's ok for ExecConstraint() to overwrite the tuple
> > descriptor of a slot "owned" by nodeResult.c. Am I missing something, or
> > is that broken?
>
> Looking into it, that partitioning code in ExecConstraint() would not
> "normally" overwrite a slot that is not managed by partitioning.
> Normally, there would be a dedicated "partition" slot that would be
> used, but only if tuple conversion from root parent to leaf partition
> is necessary before routing the tuple.
> What is not "normal" in this case is that tuple conversion is deemed
> necessary when converting from leaf partition to root parent whereas
> not in the other direction. It's because one of the partition's
> attributes has atthasmissing set to true, which triggers this code in
> convert_tuples_by_name():
Hm. I don't think we generally reach this path only with a "partition"
slot? I'm looking at 11, for the purpose of this bug. The INSERT case
"normally" is only reached with the slot returned by
ExecPrepareTupleRouting(), true. But ExecConstraint() is also
e.g. called from ExecUpdate() - and as far as I can tell that will be
either the slot from the plan, or the slot from the junkfilter.
> > It seems fairly insane how many places have this approximate code,
> > btw. In 11 we have a copy of nearly the same ~40 lines each in at least
> > ExecPartitionCheckEmitError(), ExecConstraints (twice) and
> > ExecWitchCheckOption().
>
> That is certainly bad. I think we should get that code in one place
> and inside ExecBuildSlotValueDescription() seems like a good place,
> because that's what needs a converted tuple to build a string from it.
> I have tried that in the attached -- need different patches for
> different branches as there have been a lot of small changes in this
> code over releases.
That certainly looks better.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-23 18:53 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 13+ messages in thread
From: Andres Freund @ 2020-03-23 18:53 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Daniel WM <dwilches@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi,
On 2020-03-21 23:52:17 -0700, Andres Freund wrote:
> On 2020-03-17 19:49:43 +0900, Amit Langote wrote:
> > On Sun, Mar 15, 2020 at 9:36 AM Andres Freund <andres@anarazel.de> wrote:
> > > I don't think it's ok for ExecConstraint() to overwrite the tuple
> > > descriptor of a slot "owned" by nodeResult.c. Am I missing something, or
> > > is that broken?
> >
> > Looking into it, that partitioning code in ExecConstraint() would not
> > "normally" overwrite a slot that is not managed by partitioning.
> > Normally, there would be a dedicated "partition" slot that would be
> > used, but only if tuple conversion from root parent to leaf partition
> > is necessary before routing the tuple.
>
> > What is not "normal" in this case is that tuple conversion is deemed
> > necessary when converting from leaf partition to root parent whereas
> > not in the other direction. It's because one of the partition's
> > attributes has atthasmissing set to true, which triggers this code in
> > convert_tuples_by_name():
>
> Hm. I don't think we generally reach this path only with a "partition"
> slot? I'm looking at 11, for the purpose of this bug. The INSERT case
> "normally" is only reached with the slot returned by
> ExecPrepareTupleRouting(), true. But ExecConstraint() is also
> e.g. called from ExecUpdate() - and as far as I can tell that will be
> either the slot from the plan, or the slot from the junkfilter.
Ah - it looks like we'll usually not (never?) have to do the conversion
for ExecUpdate(), because it'll execute ExecConstraints() with the leaf
partition's ResultRelInfo.
I'm a bit confused as to what this whole thing is supposed to be doing,
tbh. The explanator comment says:
/*
* If the tuple has been routed, it's been converted to the
* partition's rowtype, which might differ from the root
* table's. We must convert it back to the root table's
* rowtype so that val_desc shown error message matches the
* input tuple.
*/
but it's not clear at all why this is a good thing to be doing. For one,
the violation very well might be dependent on the child table's
definition. For another, we actually display the child table in the
error message:
SCHEMA NAME: public
TABLE NAME: other_partition
CONSTRAINT NAME: dummy_check
so it's not clear why we'd want to show the tuple in the "root" format.
And lastly, given that we're not actually showing the input tuple
(i.e. planSlot), but rather the tuple already modified by default
values, triggers, etc, I fail to understand why this is something we
should do at all.
> > > It seems fairly insane how many places have this approximate code,
> > > btw. In 11 we have a copy of nearly the same ~40 lines each in at least
> > > ExecPartitionCheckEmitError(), ExecConstraints (twice) and
> > > ExecWitchCheckOption().
> >
> > That is certainly bad. I think we should get that code in one place
> > and inside ExecBuildSlotValueDescription() seems like a good place,
> > because that's what needs a converted tuple to build a string from it.
> > I have tried that in the attached -- need different patches for
> > different branches as there have been a lot of small changes in this
> > code over releases.
>
> That certainly looks better.
One thing that concerns me with this change is that now
ExecBuildSlotValueDescription() creates a slot each time its
called. That's fine and dandy for routines that immediately throw an
error, but it's not too hard to imagine ExecBuildSlotValueDescription()
being used for other things too.
Btw, I don't think this is something we can apply to the back branches
without very good reasons - changing ExecBuildSlotValueDescription()
could break extensions. I'm only mentioning this because your cleanup
patch doesn't seem to be based on HEAD.
If we actually want to keep this conversion, I wonder if the right
answer would be to change those routines to pass in
mtstate->mt_root_tuple_slot instead (and ensure it's created when
needed). One way to achieve that would be to move
ModifyTableState->mt_root_tuple_slot to ResultRelInfo - and potentially
create it on demand?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-23 22:05 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 13+ messages in thread
From: Andres Freund @ 2020-03-23 22:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Daniel WM <dwilches@gmail.com>; pgsql-bugs@lists.postgresql.org
Hi,
On 2020-03-14 17:35:41 -0700, Andres Freund wrote:
> I think the only reason this issue doesn't have actually bad
> consequences is that we throw an error afterwards - which'll result in
> the executor tree not being used any further.
>
> I'm inclined to simply copy v12's approach of just ad-hoc creating a
> slot in those routines?
Pushed a commit doing so in 10, 11 and in a seperate commit added tests
to 10-master.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-31 06:30 Amit Langote <amitlangote09@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 13+ messages in thread
From: Amit Langote @ 2020-03-31 06:30 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Daniel WM <dwilches@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi Andres,
Thanks for the commit fixing the crash. Replying here to your
comments; sorry about the delay.
On Tue, Mar 24, 2020 at 3:53 AM Andres Freund <andres@anarazel.de> wrote:
> On 2020-03-21 23:52:17 -0700, Andres Freund wrote:
> > On 2020-03-17 19:49:43 +0900, Amit Langote wrote:
> > > On Sun, Mar 15, 2020 at 9:36 AM Andres Freund <andres@anarazel.de> wrote:
> > > > I don't think it's ok for ExecConstraint() to overwrite the tuple
> > > > descriptor of a slot "owned" by nodeResult.c. Am I missing something, or
> > > > is that broken?
> > >
> > > Looking into it, that partitioning code in ExecConstraint() would not
> > > "normally" overwrite a slot that is not managed by partitioning.
> > > Normally, there would be a dedicated "partition" slot that would be
> > > used, but only if tuple conversion from root parent to leaf partition
> > > is necessary before routing the tuple.
> >
> > > What is not "normal" in this case is that tuple conversion is deemed
> > > necessary when converting from leaf partition to root parent whereas
> > > not in the other direction. It's because one of the partition's
> > > attributes has atthasmissing set to true, which triggers this code in
> > > convert_tuples_by_name():
> >
> > Hm. I don't think we generally reach this path only with a "partition"
> > slot? I'm looking at 11, for the purpose of this bug. The INSERT case
> > "normally" is only reached with the slot returned by
> > ExecPrepareTupleRouting(), true. But ExecConstraint() is also
> > e.g. called from ExecUpdate() - and as far as I can tell that will be
> > either the slot from the plan, or the slot from the junkfilter.
>
> Ah - it looks like we'll usually not (never?) have to do the conversion
> for ExecUpdate(), because it'll execute ExecConstraints() with the leaf
> partition's ResultRelInfo.
ExecConstraints() is called with leaf partition's ResultRelInfo in any
case. Also, the tuple and the slot that it receives are always in the
leaf partition format.
In the INSERT case and the case in which UPDATE might need to use
tuple routing, we set ResultRelInfo's ri_PartitionRoot, mainly to use
for for showing root format tuples in the error messages. However, if
UPDATE doesn't need tuple routing, because the partition key is not
changed, ri_PartitionRoot is not set, so the tuples shown in the error
messages are in the leaf partition format in that case. Admittedly,
that does seem a bit inconsistemt.
> I'm a bit confused as to what this whole thing is supposed to be doing,
> tbh. The explanator comment says:
> /*
> * If the tuple has been routed, it's been converted to the
> * partition's rowtype, which might differ from the root
> * table's. We must convert it back to the root table's
> * rowtype so that val_desc shown error message matches the
> * input tuple.
> */
>
> but it's not clear at all why this is a good thing to be doing. For one,
> the violation very well might be dependent on the child table's
> definition. For another, we actually display the child table in the
> error message:
> SCHEMA NAME: public
> TABLE NAME: other_partition
> CONSTRAINT NAME: dummy_check
> so it's not clear why we'd want to show the tuple in the "root" format.
>
> And lastly, given that we're not actually showing the input tuple
> (i.e. planSlot), but rather the tuple already modified by default
> values, triggers, etc, I fail to understand why this is something we
> should do at all.
I thought at the time this code was first written that showing the
tuple in the format of the table mentioned in the query is more
user-firendly. I am fine though if we would like to change this to
show the tuple in the leaf partition format. As noted above, there
does exist a case where we show the tuple in the leaf partition format
even if it's not the table mentioned in the query.
> > > > It seems fairly insane how many places have this approximate code,
> > > > btw. In 11 we have a copy of nearly the same ~40 lines each in at least
> > > > ExecPartitionCheckEmitError(), ExecConstraints (twice) and
> > > > ExecWitchCheckOption().
> > >
> > > That is certainly bad. I think we should get that code in one place
> > > and inside ExecBuildSlotValueDescription() seems like a good place,
> > > because that's what needs a converted tuple to build a string from it.
> > > I have tried that in the attached -- need different patches for
> > > different branches as there have been a lot of small changes in this
> > > code over releases.
> >
> > That certainly looks better.
>
> One thing that concerns me with this change is that now
> ExecBuildSlotValueDescription() creates a slot each time its
> called. That's fine and dandy for routines that immediately throw an
> error, but it's not too hard to imagine ExecBuildSlotValueDescription()
> being used for other things too.
Fair point. Maybe using a fixed root slot, such as
mt_root_tuple_slot, would take care of that.
> Btw, I don't think this is something we can apply to the back branches
> without very good reasons - changing ExecBuildSlotValueDescription()
> could break extensions. I'm only mentioning this because your cleanup
> patch doesn't seem to be based on HEAD.
Hmm, ExecBuildSlotValueDescription isn't exported. Given that, I
thought it would be okay to back-patch, at least to make the code look
similar in all branches.
> If we actually want to keep this conversion, I wonder if the right
> answer would be to change those routines to pass in
> mtstate->mt_root_tuple_slot instead (and ensure it's created when
> needed). One way to achieve that would be to move
> ModifyTableState->mt_root_tuple_slot to ResultRelInfo - and potentially
> create it on demand?
Something like ri_PartitionRootSlot sounds like a good idea, but we
should make that a reference to mt_root_tuple_slot instead of its
replacement. Would you be interested in seeing a patch?
--
Thank you,
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: BUG #16293: postgres segfaults and returns SQLSTATE 08006
@ 2020-03-31 09:37 Amit Langote <amitlangote09@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 0 replies; 13+ messages in thread
From: Amit Langote @ 2020-03-31 09:37 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Daniel WM <dwilches@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Mar 31, 2020 at 3:30 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Tue, Mar 24, 2020 at 3:53 AM Andres Freund <andres@anarazel.de> wrote:
> > If we actually want to keep this conversion, I wonder if the right
> > answer would be to change those routines to pass in
> > mtstate->mt_root_tuple_slot instead (and ensure it's created when
> > needed). One way to achieve that would be to move
> > ModifyTableState->mt_root_tuple_slot to ResultRelInfo - and potentially
> > create it on demand?
>
> Something like ri_PartitionRootSlot sounds like a good idea, but we
> should make that a reference to mt_root_tuple_slot instead of its
> replacement. Would you be interested in seeing a patch?
I have updated the patch for HEAD to be this way.
--
Thank you,
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Attachments:
v2-0001-Build-root-tuple-in-ExecBuildSlotValueDescription.patch (application/octet-stream, 17.1K) download
From a67a4a8a5cc3609dd57a496be5614adee2000b5f Mon Sep 17 00:00:00 2001
From: amitlan <amitlangote09@gmail.com>
Date: Tue, 31 Mar 2020 15:36:27 +0900
Subject: [PATCH v1] Build root tuple in ExecBuildSlotValueDescription
Having the logic to do so be spread across various callers is
starting to seem cumbersome to maintain.
Use mt_root_tuple_slot to store the tuple to be shown in the
description. For it to be available in all cases that require it,
always set it in ExecInitModifyTable instead if only when UPDATE
tuple routing is active as is the case currently.
---
src/backend/executor/execMain.c | 204 ++++++++-------------------------
src/backend/executor/execPartition.c | 6 +
src/backend/executor/nodeModifyTable.c | 21 ++--
src/include/nodes/execnodes.h | 6 +
src/test/regress/expected/inherit.out | 2 +-
src/test/regress/expected/update.out | 6 +-
6 files changed, 80 insertions(+), 165 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 4fdffad..0b295c0 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -93,11 +93,10 @@ static bool ExecCheckRTEPermsModified(Oid relOid, Oid userid,
Bitmapset *modifiedCols,
AclMode requiredPerms);
static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
-static char *ExecBuildSlotValueDescription(Oid reloid,
- TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
- int maxfieldlen);
+static char *ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot,
+ int maxfieldlen);
static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
/*
@@ -1829,51 +1828,10 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- Oid root_relid;
- TupleDesc tupdesc;
char *val_desc;
- Bitmapset *modifiedCols;
- /*
- * If the tuple has been routed, it's been converted to the partition's
- * rowtype, which might differ from the root table's. We must convert it
- * back to the root table's rowtype so that val_desc in the error message
- * matches the input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc;
- AttrMap *map;
-
- root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
- tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
-
- old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(old_tupdesc, tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so allocate a
- * new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
- else
- {
- root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
- tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
- }
-
- modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
- GetUpdatedColumns(resultRelInfo, estate));
-
- val_desc = ExecBuildSlotValueDescription(root_relid,
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ val_desc = ExecBuildSlotValueDescription(estate, resultRelInfo,
+ slot, 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
@@ -1900,9 +1858,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
TupleConstr *constr = tupdesc->constr;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
Assert(constr || resultRelInfo->ri_PartitionCheck);
@@ -1918,51 +1873,19 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if (att->attnotnull && slot_attisnull(slot, attrChk))
{
char *val_desc;
- Relation orig_rel = rel;
- TupleDesc orig_tupdesc = RelationGetDescr(rel);
-
- /*
- * If the tuple has been routed, it's been converted to the
- * partition's rowtype, which might differ from the root
- * table's. We must convert it back to the root table's
- * rowtype so that val_desc shown error message matches the
- * input tuple.
- */
- if (resultRelInfo->ri_PartitionRoot)
- {
- AttrMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(orig_tupdesc,
- tupdesc);
- /*
- * Partition-specific slot's tupdesc can't be changed, so
- * allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("null value in column \"%s\" of relation \"%s\" violates not-null constraint",
NameStr(att->attname),
- RelationGetRelationName(orig_rel)),
+ RelationGetRelationName(rel)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtablecol(orig_rel, attrChk)));
+ errtablecol(rel, attrChk)));
}
}
}
@@ -1974,43 +1897,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
{
char *val_desc;
- Relation orig_rel = rel;
- /* See the comment above. */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- AttrMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(old_tupdesc,
- tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed, so
- * allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
- RelationGetRelationName(orig_rel), failed),
+ RelationGetRelationName(rel), failed),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtableconstraint(orig_rel, failed)));
+ errtableconstraint(rel, failed)));
}
}
}
@@ -2028,8 +1925,6 @@ void
ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- TupleDesc tupdesc = RelationGetDescr(rel);
ExprContext *econtext;
ListCell *l1,
*l2;
@@ -2067,9 +1962,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
if (!ExecQual(wcoExpr, econtext))
{
char *val_desc;
- Bitmapset *modifiedCols;
- Bitmapset *insertedCols;
- Bitmapset *updatedCols;
switch (wco->kind)
{
@@ -2083,34 +1975,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* USING policy.
*/
case WCO_VIEW_CHECK:
- /* See the comment in ExecConstraints(). */
- if (resultRelInfo->ri_PartitionRoot)
- {
- TupleDesc old_tupdesc = RelationGetDescr(rel);
- AttrMap *map;
-
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
- /* a reverse map */
- map = build_attrmap_by_name_if_req(old_tupdesc,
- tupdesc);
-
- /*
- * Partition-specific slot's tupdesc can't be changed,
- * so allocate a new one.
- */
- if (map != NULL)
- slot = execute_attr_map_slot(map, slot,
- MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
- }
-
- insertedCols = GetInsertedColumns(resultRelInfo, estate);
- updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
+ val_desc = ExecBuildSlotValueDescription(estate,
+ resultRelInfo,
slot,
- tupdesc,
- modifiedCols,
64);
ereport(ERROR,
@@ -2163,8 +2030,7 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
*
* Also, unlike the case with index entries, we need to be prepared to ignore
* dropped columns. We used to use the slot's tuple descriptor to decode the
- * data, but the slot's descriptor doesn't identify dropped columns, so we
- * now need to be passed the relation's descriptor.
+ * data, but the slot's descriptor doesn't identify dropped columns.
*
* Note that, like BuildIndexValueDescription, if the user does not have
* permission to view any of the columns involved, a NULL is returned. Unlike
@@ -2173,10 +2039,9 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
* columns they are.
*/
static char *
-ExecBuildSlotValueDescription(Oid reloid,
+ExecBuildSlotValueDescription(EState *estate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
- TupleDesc tupdesc,
- Bitmapset *modifiedCols,
int maxfieldlen)
{
StringInfoData buf;
@@ -2187,6 +2052,9 @@ ExecBuildSlotValueDescription(Oid reloid,
AclResult aclresult;
bool table_perm = false;
bool any_perm = false;
+ Oid reloid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ TupleDesc tupdesc;
+ Bitmapset *modifiedCols;
/*
* Check if RLS is enabled and should be active for the relation; if so,
@@ -2196,6 +2064,34 @@ ExecBuildSlotValueDescription(Oid reloid,
if (check_enable_rls(reloid, InvalidOid, true) == RLS_ENABLED)
return NULL;
+ /*
+ * If the tuple has been routed, it's been converted to the partition's
+ * rowtype, which might differ from the root table's. We must convert it
+ * back to the root table's rowtype so that val_desc matches the input
+ * tuple.
+ */
+ if (resultRelInfo->ri_PartitionRootSlot)
+ {
+ TupleDesc old_tupdesc;
+ AttrMap *map;
+
+ tupdesc = resultRelInfo->ri_PartitionRootSlot->tts_tupleDescriptor;
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ /* a reverse map */
+ map = build_attrmap_by_name_if_req(old_tupdesc, tupdesc);
+
+ /* Note that this uses a new slot for the converted tuple. */
+ if (map != NULL)
+ slot = execute_attr_map_slot(map, slot,
+ resultRelInfo->ri_PartitionRootSlot);
+ }
+ else
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+
+ modifiedCols = bms_union(GetInsertedColumns(resultRelInfo, estate),
+ GetUpdatedColumns(resultRelInfo, estate));
+
initStringInfo(&buf);
appendStringInfoChar(&buf, '(');
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index fb6ce49..9069c5b 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -527,6 +527,12 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
estate->es_instrument);
/*
+ * ri_PartitionRootSlot will be used when converting partition's tuples to
+ * root format.
+ */
+ leaf_part_rri->ri_PartitionRootSlot = mtstate->mt_root_tuple_slot;
+
+ /*
* Verify result relation is a valid target for an INSERT. An UPDATE of a
* partition-key becomes a DELETE+INSERT operation, so this check is still
* required when the operation is CMD_UPDATE.
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index d71c0a4..9369ee7 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2320,8 +2320,14 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/* If modifying a partitioned table, initialize the root table info */
if (node->rootResultRelIndex >= 0)
+ {
+ Relation rootrel;
+
mtstate->rootResultRelInfo = estate->es_root_result_relations +
node->rootResultRelIndex;
+ rootrel = mtstate->rootResultRelInfo->ri_RelationDesc;
+ mtstate->mt_root_tuple_slot = table_slot_create(rootrel, NULL);
+ }
mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
mtstate->mt_nplans = nplans;
@@ -2403,6 +2409,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
eflags);
}
+ /*
+ * ri_PartitionRootSlot will be used when converting partition's
+ * tuples to root format.
+ */
+ resultRelInfo->ri_PartitionRootSlot = mtstate->mt_root_tuple_slot;
+
resultRelInfo++;
i++;
}
@@ -2445,10 +2457,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
* We can skip this setup if it's not a partition key update.
*/
if (update_tuple_routing_needed)
- {
ExecSetupChildParentMapForSubplan(mtstate);
- mtstate->mt_root_tuple_slot = table_slot_create(rel, NULL);
- }
/*
* Initialize any WITH CHECK OPTION constraints if needed.
@@ -2774,12 +2783,10 @@ ExecEndModifyTable(ModifyTableState *node)
* and release the slot used for tuple routing, if set.
*/
if (node->mt_partition_tuple_routing)
- {
ExecCleanupTupleRouting(node, node->mt_partition_tuple_routing);
- if (node->mt_root_tuple_slot)
- ExecDropSingleTupleTableSlot(node->mt_root_tuple_slot);
- }
+ if (node->mt_root_tuple_slot)
+ ExecDropSingleTupleTableSlot(node->mt_root_tuple_slot);
/*
* Free the exprcontext
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 0fb5d61..fbcff6a 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -486,6 +486,12 @@ typedef struct ResultRelInfo
/* relation descriptor for root partitioned table */
Relation ri_PartitionRoot;
+ /*
+ * Slot initialized with root table descriptor; may be set even if
+ * ri_PartitionRoot is not.
+ */
+ TupleTableSlot *ri_PartitionRootSlot;
+
/* Additional information specific to partition tuple routing */
struct PartitionRoutingInfo *ri_PartitionInfo;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 2b68aef..fbfff7b 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2494,7 +2494,7 @@ ERROR: new row for relation "errtst_child_plaindef" violates check constraint "
DETAIL: Failing row contains (10, 1, 15).
UPDATE errtst_parent SET data = data + 10 WHERE partid = 20;
ERROR: new row for relation "errtst_child_reorder" violates check constraint "errtst_child_reorder_data_check"
-DETAIL: Failing row contains (15, 1, 20).
+DETAIL: Failing row contains (20, 1, 15).
-- direct leaf partition update, without partition id violation
BEGIN;
UPDATE errtst_child_fastdef SET partid = 1 WHERE partid = 0;
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index b7f90de..91a8280 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -344,7 +344,7 @@ DETAIL: Failing row contains (105, 85, null, b, 15).
-- but "a = 'a'" violates partition constraint enforced by root partition)
UPDATE part_b_10_b_20 set a = 'a';
ERROR: new row for relation "part_c_1_100" violates partition constraint
-DETAIL: Failing row contains (null, 1, 96, 12, a).
+DETAIL: Failing row contains (null, 96, a, 12, 1).
-- ok, partition key update, no constraint violation
UPDATE range_parted set d = d - 10 WHERE d > 10;
-- ok, no partition key update, no constraint violation
@@ -375,7 +375,7 @@ UPDATE part_b_10_b_20 set c = c + 20 returning c, b, a;
-- fail, row movement happens only within the partition subtree.
UPDATE part_b_10_b_20 set b = b - 6 WHERE c > 116 returning *;
ERROR: new row for relation "part_d_1_15" violates partition constraint
-DETAIL: Failing row contains (2, 117, 2, b, 7).
+DETAIL: Failing row contains (2, 117, b, 7, 2).
-- ok, row movement, with subset of rows moved into different partition.
UPDATE range_parted set b = b - 6 WHERE c > 116 returning a, b + c;
a | ?column?
@@ -817,7 +817,7 @@ INSERT into sub_parted VALUES (1,2,10);
-- constraint is inherited from upper root.
UPDATE sub_parted set a = 2 WHERE c = 10;
ERROR: new row for relation "sub_part2" violates partition constraint
-DETAIL: Failing row contains (2, 10, 2).
+DETAIL: Failing row contains (2, 2, 10).
-- Test update-partition-key, where the unpruned partitions do not have their
-- partition keys updated.
SELECT tableoid::regclass::text, * FROM list_parted WHERE a = 2 ORDER BY 1;
--
1.8.3.1
end of thread, other threads:[~2020-03-31 09:37 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10 21:13 BUG #16293: postgres segfaults and returns SQLSTATE 08006 PG Bug reporting form <noreply@postgresql.org>
2020-03-10 22:19 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-03-10 23:04 ` Daniel WM <dwilches@gmail.com>
2020-03-14 20:43 ` Daniel WM <dwilches@gmail.com>
2020-03-14 21:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-03-14 21:26 ` Andres Freund <andres@anarazel.de>
2020-03-15 00:35 ` Andres Freund <andres@anarazel.de>
2020-03-17 10:49 ` Amit Langote <amitlangote09@gmail.com>
2020-03-22 06:52 ` Andres Freund <andres@anarazel.de>
2020-03-23 18:53 ` Andres Freund <andres@anarazel.de>
2020-03-31 06:30 ` Amit Langote <amitlangote09@gmail.com>
2020-03-31 09:37 ` Amit Langote <amitlangote09@gmail.com>
2020-03-23 22:05 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox