On Fri, Dec 23, 2022 at 1:04 PM Richard Guo <guofengli...@gmail.com> wrote: > On Fri, Dec 23, 2022 at 11:22 AM Amit Langote <amitlangot...@gmail.com> wrote: >> Attached shows a test case I was able to come up with that I can see >> is broken by a61b1f74 though passes after applying Richard's patch. >> What's broken is that deparseUpdateSql() outputs a remote UPDATE >> statement with the wrong SET column list, because the wrong attribute >> numbers would be added to the targetAttrs list by the above code block >> after the buggy multi-level translation in ger_rel_all_updated_cols(). > > Thanks for the test! I looked at this and found that with WCO > constraints we can also hit the buggy code.
Ah, yes. /* * Try to modify the foreign table directly if (1) the FDW provides * callback functions needed for that and (2) there are no local * structures that need to be run for each modified row: row-level * triggers on the foreign table, stored generated columns, WITH CHECK * OPTIONs from parent views. */ direct_modify = false; if (fdwroutine != NULL && fdwroutine->PlanDirectModify != NULL && fdwroutine->BeginDirectModify != NULL && fdwroutine->IterateDirectModify != NULL && fdwroutine->EndDirectModify != NULL && withCheckOptionLists == NIL && !has_row_triggers(root, rti, operation) && !has_stored_generated_columns(root, rti)) direct_modify = fdwroutine->PlanDirectModify(root, node, rti, i); > Based on David's test case, > I came up with the following in the morning. > > CREATE FOREIGN TABLE ft_gc ( > a int, > b int, > c int > ) SERVER loopback OPTIONS (schema_name 'public', table_name 't_gc'); > > alter table t_c attach partition ft_gc for values in (1); > alter table t_tlp attach partition t_c for values in (1); > > CREATE VIEW rw_view AS SELECT * FROM t_tlp where a < b WITH CHECK OPTION; > > explain (verbose, costs off) update rw_view set c = 42; > > Currently on HEAD, we can see something wrong in the plan. > > QUERY PLAN > -------------------------------------------------------------------------------------- > Update on public.t_tlp > Foreign Update on public.ft_gc t_tlp_1 > Remote SQL: UPDATE public.t_gc SET b = $2 WHERE ctid = $1 RETURNING a, b > -> Foreign Scan on public.ft_gc t_tlp_1 > Output: 42, t_tlp_1.tableoid, t_tlp_1.ctid, t_tlp_1.* > Remote SQL: SELECT a, b, c, ctid FROM public.t_gc WHERE ((a < b)) > FOR UPDATE > (6 rows) > > Note that this is wrong: 'UPDATE public.t_gc SET b = $2'. Right, because of the mangled targetAttrs in postgresPlanForeignModify(). -- Thanks, Amit Langote EDB: http://www.enterprisedb.com