Skip to content

Commit fc02019

Browse files
author
Etsuro Fujita
committed
Fix handling of pending inserts in nodeModifyTable.c.
Commit b663a41, which allowed FDWs to INSERT rows in bulk, added to nodeModifyTable.c code to flush pending inserts to the foreign-table result relation(s) before completing processing of the ModifyTable node, but the code failed to take into account the case where the INSERT query has modifying CTEs, leading to incorrect results. Also, that commit failed to flush pending inserts before firing BEFORE ROW triggers so that rows are visible to such triggers. In that commit we scanned through EState's es_tuple_routing_result_relations or es_opened_result_relations list to find the foreign-table result relations to which pending inserts are flushed, but that would be inefficient in some cases. So to fix, 1) add a List member to EState to record the insert-pending result relations, and 2) modify nodeModifyTable.c so that it adds the foreign-table result relation to the list in ExecInsert() if appropriate, and flushes pending inserts properly using the list where needed. While here, fix a copy-and-pasteo in a comment in ExecBatchInsert(), which was added by that commit. Back-patch to v14 where that commit appeared. Discussion: https://postgr.es/m/CAPmGK16qutyCmyJJzgQOhfBq%3DNoGDqTB6O0QBZTihrbqre%2BoxA%40mail.gmail.com
1 parent 898ef41 commit fc02019

File tree

7 files changed

+302
-21
lines changed

7 files changed

+302
-21
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10316,7 +10316,130 @@ SELECT * FROM batch_table ORDER BY x;
1031610316
50 | test50 | test50
1031710317
(50 rows)
1031810318

10319+
-- Clean up
10320+
DROP TABLE batch_table;
10321+
DROP TABLE batch_table_p0;
10322+
DROP TABLE batch_table_p1;
1031910323
ALTER SERVER loopback OPTIONS (DROP batch_size);
10324+
-- Test that pending inserts are handled properly when needed
10325+
CREATE TABLE batch_table (a text, b int);
10326+
CREATE FOREIGN TABLE ftable (a text, b int)
10327+
SERVER loopback
10328+
OPTIONS (table_name 'batch_table', batch_size '2');
10329+
CREATE TABLE ltable (a text, b int);
10330+
CREATE FUNCTION ftable_rowcount_trigf() RETURNS trigger LANGUAGE plpgsql AS
10331+
$$
10332+
begin
10333+
raise notice '%: there are % rows in ftable',
10334+
TG_NAME, (SELECT count(*) FROM ftable);
10335+
if TG_OP = 'DELETE' then
10336+
return OLD;
10337+
else
10338+
return NEW;
10339+
end if;
10340+
end;
10341+
$$;
10342+
CREATE TRIGGER ftable_rowcount_trigger
10343+
BEFORE INSERT OR UPDATE OR DELETE ON ltable
10344+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
10345+
WITH t AS (
10346+
INSERT INTO ltable VALUES ('AAA', 42), ('BBB', 42) RETURNING *
10347+
)
10348+
INSERT INTO ftable SELECT * FROM t;
10349+
NOTICE: ftable_rowcount_trigger: there are 0 rows in ftable
10350+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10351+
SELECT * FROM ltable;
10352+
a | b
10353+
-----+----
10354+
AAA | 42
10355+
BBB | 42
10356+
(2 rows)
10357+
10358+
SELECT * FROM ftable;
10359+
a | b
10360+
-----+----
10361+
AAA | 42
10362+
BBB | 42
10363+
(2 rows)
10364+
10365+
DELETE FROM ftable;
10366+
WITH t AS (
10367+
UPDATE ltable SET b = b + 100 RETURNING *
10368+
)
10369+
INSERT INTO ftable SELECT * FROM t;
10370+
NOTICE: ftable_rowcount_trigger: there are 0 rows in ftable
10371+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10372+
SELECT * FROM ltable;
10373+
a | b
10374+
-----+-----
10375+
AAA | 142
10376+
BBB | 142
10377+
(2 rows)
10378+
10379+
SELECT * FROM ftable;
10380+
a | b
10381+
-----+-----
10382+
AAA | 142
10383+
BBB | 142
10384+
(2 rows)
10385+
10386+
DELETE FROM ftable;
10387+
WITH t AS (
10388+
DELETE FROM ltable RETURNING *
10389+
)
10390+
INSERT INTO ftable SELECT * FROM t;
10391+
NOTICE: ftable_rowcount_trigger: there are 0 rows in ftable
10392+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10393+
SELECT * FROM ltable;
10394+
a | b
10395+
---+---
10396+
(0 rows)
10397+
10398+
SELECT * FROM ftable;
10399+
a | b
10400+
-----+-----
10401+
AAA | 142
10402+
BBB | 142
10403+
(2 rows)
10404+
10405+
DELETE FROM ftable;
10406+
-- Clean up
10407+
DROP FOREIGN TABLE ftable;
10408+
DROP TABLE batch_table;
10409+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
10410+
DROP TABLE ltable;
10411+
CREATE TABLE parent (a text, b int) PARTITION BY LIST (a);
10412+
CREATE TABLE batch_table (a text, b int);
10413+
CREATE FOREIGN TABLE ftable
10414+
PARTITION OF parent
10415+
FOR VALUES IN ('AAA')
10416+
SERVER loopback
10417+
OPTIONS (table_name 'batch_table', batch_size '2');
10418+
CREATE TABLE ltable
10419+
PARTITION OF parent
10420+
FOR VALUES IN ('BBB');
10421+
CREATE TRIGGER ftable_rowcount_trigger
10422+
BEFORE INSERT ON ltable
10423+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
10424+
INSERT INTO parent VALUES ('AAA', 42), ('BBB', 42), ('AAA', 42), ('BBB', 42);
10425+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10426+
NOTICE: ftable_rowcount_trigger: there are 2 rows in ftable
10427+
SELECT tableoid::regclass, * FROM parent;
10428+
tableoid | a | b
10429+
----------+-----+----
10430+
ftable | AAA | 42
10431+
ftable | AAA | 42
10432+
ltable | BBB | 42
10433+
ltable | BBB | 42
10434+
(4 rows)
10435+
10436+
-- Clean up
10437+
DROP FOREIGN TABLE ftable;
10438+
DROP TABLE batch_table;
10439+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
10440+
DROP TABLE ltable;
10441+
DROP TABLE parent;
10442+
DROP FUNCTION ftable_rowcount_trigf;
1032010443
-- ===================================================================
1032110444
-- test asynchronous execution
1032210445
-- ===================================================================

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3261,8 +3261,94 @@ INSERT INTO batch_table SELECT i, 'test'||i, 'test'|| i FROM generate_series(1,
32613261
SELECT COUNT(*) FROM batch_table;
32623262
SELECT * FROM batch_table ORDER BY x;
32633263

3264+
-- Clean up
3265+
DROP TABLE batch_table;
3266+
DROP TABLE batch_table_p0;
3267+
DROP TABLE batch_table_p1;
3268+
32643269
ALTER SERVER loopback OPTIONS (DROP batch_size);
32653270

3271+
-- Test that pending inserts are handled properly when needed
3272+
CREATE TABLE batch_table (a text, b int);
3273+
CREATE FOREIGN TABLE ftable (a text, b int)
3274+
SERVER loopback
3275+
OPTIONS (table_name 'batch_table', batch_size '2');
3276+
CREATE TABLE ltable (a text, b int);
3277+
CREATE FUNCTION ftable_rowcount_trigf() RETURNS trigger LANGUAGE plpgsql AS
3278+
$$
3279+
begin
3280+
raise notice '%: there are % rows in ftable',
3281+
TG_NAME, (SELECT count(*) FROM ftable);
3282+
if TG_OP = 'DELETE' then
3283+
return OLD;
3284+
else
3285+
return NEW;
3286+
end if;
3287+
end;
3288+
$$;
3289+
CREATE TRIGGER ftable_rowcount_trigger
3290+
BEFORE INSERT OR UPDATE OR DELETE ON ltable
3291+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
3292+
3293+
WITH t AS (
3294+
INSERT INTO ltable VALUES ('AAA', 42), ('BBB', 42) RETURNING *
3295+
)
3296+
INSERT INTO ftable SELECT * FROM t;
3297+
3298+
SELECT * FROM ltable;
3299+
SELECT * FROM ftable;
3300+
DELETE FROM ftable;
3301+
3302+
WITH t AS (
3303+
UPDATE ltable SET b = b + 100 RETURNING *
3304+
)
3305+
INSERT INTO ftable SELECT * FROM t;
3306+
3307+
SELECT * FROM ltable;
3308+
SELECT * FROM ftable;
3309+
DELETE FROM ftable;
3310+
3311+
WITH t AS (
3312+
DELETE FROM ltable RETURNING *
3313+
)
3314+
INSERT INTO ftable SELECT * FROM t;
3315+
3316+
SELECT * FROM ltable;
3317+
SELECT * FROM ftable;
3318+
DELETE FROM ftable;
3319+
3320+
-- Clean up
3321+
DROP FOREIGN TABLE ftable;
3322+
DROP TABLE batch_table;
3323+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
3324+
DROP TABLE ltable;
3325+
3326+
CREATE TABLE parent (a text, b int) PARTITION BY LIST (a);
3327+
CREATE TABLE batch_table (a text, b int);
3328+
CREATE FOREIGN TABLE ftable
3329+
PARTITION OF parent
3330+
FOR VALUES IN ('AAA')
3331+
SERVER loopback
3332+
OPTIONS (table_name 'batch_table', batch_size '2');
3333+
CREATE TABLE ltable
3334+
PARTITION OF parent
3335+
FOR VALUES IN ('BBB');
3336+
CREATE TRIGGER ftable_rowcount_trigger
3337+
BEFORE INSERT ON ltable
3338+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
3339+
3340+
INSERT INTO parent VALUES ('AAA', 42), ('BBB', 42), ('AAA', 42), ('BBB', 42);
3341+
3342+
SELECT tableoid::regclass, * FROM parent;
3343+
3344+
-- Clean up
3345+
DROP FOREIGN TABLE ftable;
3346+
DROP TABLE batch_table;
3347+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
3348+
DROP TABLE ltable;
3349+
DROP TABLE parent;
3350+
DROP FUNCTION ftable_rowcount_trigf;
3351+
32663352
-- ===================================================================
32673353
-- test asynchronous execution
32683354
-- ===================================================================

‎src/backend/executor/execMain.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
12611261
resultRelInfo->ri_ChildToRootMap = NULL;
12621262
resultRelInfo->ri_ChildToRootMapValid = false;
12631263
resultRelInfo->ri_CopyMultiInsertBuffer = NULL;
1264+
resultRelInfo->ri_ModifyTableState = NULL;
12641265
}
12651266

12661267
/*

‎src/backend/executor/execPartition.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,13 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,
10351035

10361036
Assert(partRelInfo->ri_BatchSize >= 1);
10371037

1038+
/*
1039+
* If doing batch insert, setup back-link so we can easily find the
1040+
* mtstate again.
1041+
*/
1042+
if (partRelInfo->ri_BatchSize > 1)
1043+
partRelInfo->ri_ModifyTableState = mtstate;
1044+
10381045
partRelInfo->ri_CopyMultiInsertBuffer = NULL;
10391046

10401047
/*

‎src/backend/executor/execUtils.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ CreateExecutorState(void)
127127
estate->es_result_relations = NULL;
128128
estate->es_opened_result_relations = NIL;
129129
estate->es_tuple_routing_result_relations = NIL;
130+
estate->es_insert_pending_result_relations = NIL;
130131
estate->es_trig_target_relations = NIL;
131132

132133
estate->es_param_list_info = NULL;

0 commit comments

Comments
 (0)