Re: Avoid lost result of recursion (src/backend/optimizer/util/inherit.c)

On Fri, Dec 23, 2022 at 1:04 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> On Fri, Dec 23, 2022 at 11:22 AM Amit Langote <amitlangote09(at)gmail(dot)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