Skip to content

Commit ffbb7e6

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 9e492d6 commit ffbb7e6

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
@@ -10448,7 +10448,130 @@ SELECT * FROM batch_table ORDER BY x;
1044810448
50 | test50 | test50
1044910449
(50 rows)
1045010450

10451+
-- Clean up
10452+
DROP TABLE batch_table;
10453+
DROP TABLE batch_table_p0;
10454+
DROP TABLE batch_table_p1;
1045110455
ALTER SERVER loopback OPTIONS (DROP batch_size);
10456+
-- Test that pending inserts are handled properly when needed
10457+
CREATE TABLE batch_table (a text, b int);
10458+
CREATE FOREIGN TABLE ftable (a text, b int)
10459+
SERVER loopback
10460+
OPTIONS (table_name 'batch_table', batch_size '2');
10461+
CREATE TABLE ltable (a text, b int);
10462+
CREATE FUNCTION ftable_rowcount_trigf() RETURNS trigger LANGUAGE plpgsql AS
10463+
$$
10464+
begin
10465+
raise notice '%: there are % rows in ftable',
10466+
TG_NAME, (SELECT count(*) FROM ftable);
10467+
if TG_OP = 'DELETE' then
10468+
return OLD;
10469+
else
10470+
return NEW;
10471+
end if;
10472+
end;
10473+
$$;
10474+
CREATE TRIGGER ftable_rowcount_trigger
10475+
BEFORE INSERT OR UPDATE OR DELETE ON ltable
10476+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
10477+
WITH t AS (
10478+
INSERT INTO ltable VALUES ('AAA', 42), ('BBB', 42) RETURNING *
10479+
)
10480+
INSERT INTO ftable SELECT * FROM t;
10481+
NOTICE: ftable_rowcount_trigger: there are 0 rows in ftable
10482+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10483+
SELECT * FROM ltable;
10484+
a | b
10485+
-----+----
10486+
AAA | 42
10487+
BBB | 42
10488+
(2 rows)
10489+
10490+
SELECT * FROM ftable;
10491+
a | b
10492+
-----+----
10493+
AAA | 42
10494+
BBB | 42
10495+
(2 rows)
10496+
10497+
DELETE FROM ftable;
10498+
WITH t AS (
10499+
UPDATE ltable SET b = b + 100 RETURNING *
10500+
)
10501+
INSERT INTO ftable SELECT * FROM t;
10502+
NOTICE: ftable_rowcount_trigger: there are 0 rows in ftable
10503+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10504+
SELECT * FROM ltable;
10505+
a | b
10506+
-----+-----
10507+
AAA | 142
10508+
BBB | 142
10509+
(2 rows)
10510+
10511+
SELECT * FROM ftable;
10512+
a | b
10513+
-----+-----
10514+
AAA | 142
10515+
BBB | 142
10516+
(2 rows)
10517+
10518+
DELETE FROM ftable;
10519+
WITH t AS (
10520+
DELETE FROM ltable RETURNING *
10521+
)
10522+
INSERT INTO ftable SELECT * FROM t;
10523+
NOTICE: ftable_rowcount_trigger: there are 0 rows in ftable
10524+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10525+
SELECT * FROM ltable;
10526+
a | b
10527+
---+---
10528+
(0 rows)
10529+
10530+
SELECT * FROM ftable;
10531+
a | b
10532+
-----+-----
10533+
AAA | 142
10534+
BBB | 142
10535+
(2 rows)
10536+
10537+
DELETE FROM ftable;
10538+
-- Clean up
10539+
DROP FOREIGN TABLE ftable;
10540+
DROP TABLE batch_table;
10541+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
10542+
DROP TABLE ltable;
10543+
CREATE TABLE parent (a text, b int) PARTITION BY LIST (a);
10544+
CREATE TABLE batch_table (a text, b int);
10545+
CREATE FOREIGN TABLE ftable
10546+
PARTITION OF parent
10547+
FOR VALUES IN ('AAA')
10548+
SERVER loopback
10549+
OPTIONS (table_name 'batch_table', batch_size '2');
10550+
CREATE TABLE ltable
10551+
PARTITION OF parent
10552+
FOR VALUES IN ('BBB');
10553+
CREATE TRIGGER ftable_rowcount_trigger
10554+
BEFORE INSERT ON ltable
10555+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
10556+
INSERT INTO parent VALUES ('AAA', 42), ('BBB', 42), ('AAA', 42), ('BBB', 42);
10557+
NOTICE: ftable_rowcount_trigger: there are 1 rows in ftable
10558+
NOTICE: ftable_rowcount_trigger: there are 2 rows in ftable
10559+
SELECT tableoid::regclass, * FROM parent;
10560+
tableoid | a | b
10561+
----------+-----+----
10562+
ftable | AAA | 42
10563+
ftable | AAA | 42
10564+
ltable | BBB | 42
10565+
ltable | BBB | 42
10566+
(4 rows)
10567+
10568+
-- Clean up
10569+
DROP FOREIGN TABLE ftable;
10570+
DROP TABLE batch_table;
10571+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
10572+
DROP TABLE ltable;
10573+
DROP TABLE parent;
10574+
DROP FUNCTION ftable_rowcount_trigf;
1045210575
-- ===================================================================
1045310576
-- test asynchronous execution
1045410577
-- ===================================================================

‎contrib/postgres_fdw/sql/postgres_fdw.sql

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

3375+
-- Clean up
3376+
DROP TABLE batch_table;
3377+
DROP TABLE batch_table_p0;
3378+
DROP TABLE batch_table_p1;
3379+
33753380
ALTER SERVER loopback OPTIONS (DROP batch_size);
33763381

3382+
-- Test that pending inserts are handled properly when needed
3383+
CREATE TABLE batch_table (a text, b int);
3384+
CREATE FOREIGN TABLE ftable (a text, b int)
3385+
SERVER loopback
3386+
OPTIONS (table_name 'batch_table', batch_size '2');
3387+
CREATE TABLE ltable (a text, b int);
3388+
CREATE FUNCTION ftable_rowcount_trigf() RETURNS trigger LANGUAGE plpgsql AS
3389+
$$
3390+
begin
3391+
raise notice '%: there are % rows in ftable',
3392+
TG_NAME, (SELECT count(*) FROM ftable);
3393+
if TG_OP = 'DELETE' then
3394+
return OLD;
3395+
else
3396+
return NEW;
3397+
end if;
3398+
end;
3399+
$$;
3400+
CREATE TRIGGER ftable_rowcount_trigger
3401+
BEFORE INSERT OR UPDATE OR DELETE ON ltable
3402+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
3403+
3404+
WITH t AS (
3405+
INSERT INTO ltable VALUES ('AAA', 42), ('BBB', 42) RETURNING *
3406+
)
3407+
INSERT INTO ftable SELECT * FROM t;
3408+
3409+
SELECT * FROM ltable;
3410+
SELECT * FROM ftable;
3411+
DELETE FROM ftable;
3412+
3413+
WITH t AS (
3414+
UPDATE ltable SET b = b + 100 RETURNING *
3415+
)
3416+
INSERT INTO ftable SELECT * FROM t;
3417+
3418+
SELECT * FROM ltable;
3419+
SELECT * FROM ftable;
3420+
DELETE FROM ftable;
3421+
3422+
WITH t AS (
3423+
DELETE FROM ltable RETURNING *
3424+
)
3425+
INSERT INTO ftable SELECT * FROM t;
3426+
3427+
SELECT * FROM ltable;
3428+
SELECT * FROM ftable;
3429+
DELETE FROM ftable;
3430+
3431+
-- Clean up
3432+
DROP FOREIGN TABLE ftable;
3433+
DROP TABLE batch_table;
3434+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
3435+
DROP TABLE ltable;
3436+
3437+
CREATE TABLE parent (a text, b int) PARTITION BY LIST (a);
3438+
CREATE TABLE batch_table (a text, b int);
3439+
CREATE FOREIGN TABLE ftable
3440+
PARTITION OF parent
3441+
FOR VALUES IN ('AAA')
3442+
SERVER loopback
3443+
OPTIONS (table_name 'batch_table', batch_size '2');
3444+
CREATE TABLE ltable
3445+
PARTITION OF parent
3446+
FOR VALUES IN ('BBB');
3447+
CREATE TRIGGER ftable_rowcount_trigger
3448+
BEFORE INSERT ON ltable
3449+
FOR EACH ROW EXECUTE PROCEDURE ftable_rowcount_trigf();
3450+
3451+
INSERT INTO parent VALUES ('AAA', 42), ('BBB', 42), ('AAA', 42), ('BBB', 42);
3452+
3453+
SELECT tableoid::regclass, * FROM parent;
3454+
3455+
-- Clean up
3456+
DROP FOREIGN TABLE ftable;
3457+
DROP TABLE batch_table;
3458+
DROP TRIGGER ftable_rowcount_trigger ON ltable;
3459+
DROP TABLE ltable;
3460+
DROP TABLE parent;
3461+
DROP FUNCTION ftable_rowcount_trigf;
3462+
33773463
-- ===================================================================
33783464
-- test asynchronous execution
33793465
-- ===================================================================

‎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
@@ -1034,6 +1034,13 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,
10341034

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

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

10391046
/*

‎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)