Ajit Awekar <[email protected]> wrote: > Verified against your repros plus the full regression.
I tested v2 (cfbot branch for CF 6770, on master at 862092932c9) against
the same master without it, both with --enable-cassert, using a loopback
server. The attached run_cases.sh runs each case file on a fresh
cluster.
What v2 fixes
-------------
- Etsuro's DELETE ... USING case: master deletes 20 rows instead of 10;
with v2, exactly the 10 matching rows.
- The same corruption through triggers (case_d): remote partitioned
table, foreign table with a dropped column in the middle, a column
mapped with column_name, a different column order, a BEFORE UPDATE
trigger changing NEW and an AFTER UPDATE trigger reading OLD. On
master, updating ids 1 and 2 leaves two rows with id 2 and none with
id 1. With v2 both rows are right, and OLD and NEW seen by the
triggers are right, so the local whole-row reconstruction handles the
dropped and renamed columns.
- A local partitioned table with one local and one foreign partition,
UPDATE and DELETE with RETURNING tableoid and the whole row (case_g),
and a self-join UPDATE (case_h): wrong rows on master, right with v2.
Jakub's zero-column case with a BEFORE DELETE trigger still fails the
assertion (postgres_fdw.c:9281 on this tree), while master runs it
fine. Without the trigger, DELETE ... RETURNING ft1 on the same table
works on both.
What v2 does not fix: chained foreign tables
--------------------------------------------
When a foreign table points to another foreign table, which points to
the partitioned table, the wrong rows are still updated and deleted,
exactly as on master (case_f):
CREATE TABLE r (id int, grp int, v text) PARTITION BY LIST (grp);
CREATE TABLE r1 PARTITION OF r FOR VALUES IN (1);
CREATE TABLE r2 PARTITION OF r FOR VALUES IN (2);
INSERT INTO r VALUES (1, 1, 'one'), (2, 2, 'two');
CREATE FOREIGN TABLE ft_mid (id int, grp int, v text)
SERVER loopback OPTIONS (table_name 'r');
CREATE FOREIGN TABLE ft_outer (id int, grp int, v text)
SERVER loopback OPTIONS (table_name 'ft_mid');
UPDATE ft_outer SET v = v || '!' WHERE id = 1 AND random() <= 1;
SELECT tableoid::regclass, ctid, * FROM r ORDER BY id;
tableoid | ctid | id | grp | v
----------+-------+----+-----+------
r1 | (0,2) | 1 | 1 | one!
r2 | (0,2) | 2 | 2 | one!
With log_statement = all the cause is visible. The outer hop sends
UPDATE public.ft_mid SET v = $3 WHERE ctid = $1 AND tableoid = $2
Parameters: $1 = '(0,1)', $2 = '16408', $3 = 'one!'
where 16408 is ft_mid itself, so the tableoid condition matches every
row of ft_mid. The middle hop then finds (0,1) in both partitions and
sends two updates, one per remote tableoid:
UPDATE public.r SET v = $3 WHERE ctid = $1 AND tableoid = $2
Parameters: $1 = '(0,1)', $2 = '16398', $3 = 'one!'
UPDATE public.r SET v = $3 WHERE ctid = $1 AND tableoid = $2
Parameters: $1 = '(0,1)', $2 = '16403', $3 = 'one!'
In the same case, DELETE ... WHERE id = 3 also deletes id 4.
So once the remote table is itself a foreign table, (ctid, tableoid)
is no longer a row identity. I don't see how the outer hop could fix
that without knowing the remote relkind. Maybe that is worth a
sentence next to the new partitioning note in postgres-fdw.sgml, or a
check of the remote relkind that refuses the non-direct path for it,
but I would rather ask what you think than propose one. The EXPLAIN
output in this case now shows "WHERE ctid = $1 AND tableoid = $2",
which makes it look protected when it is not.
Regards,
Manu
run_cases.sh
Description: application/shellscript
case_f_chained_fdw.sql
Description: application/sql
case_d_dropped_and_renamed_columns.sql
Description: application/sql
case_g_mixed_local_foreign_partitions.sql
Description: application/sql
case_h_self_join.sql
Description: application/sql
