Re: Problem while updating a foreign table pointing to a partitionedtable on foreign server - Mailing list pgsql-hackers
From | Etsuro Fujita |
---|---|
Subject | Re: Problem while updating a foreign table pointing to a partitionedtable on foreign server |
Date | |
Msg-id | 5B87E9DC.3050100@lab.ntt.co.jp Whole thread Raw |
In response to | Re: Problem while updating a foreign table pointing to apartitioned table on foreign server (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Responses |
Re: Problem while updating a foreign table pointing to a partitionedtable on foreign server
|
List | pgsql-hackers |
(2018/08/30 20:37), Kyotaro HORIGUCHI wrote: > At Fri, 24 Aug 2018 21:45:35 +0900, Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp> wrote in<5B7FFDEF.6020302@lab.ntt.co.jp> >> (2018/08/21 11:01), Kyotaro HORIGUCHI wrote: >>> At Tue, 14 Aug 2018 20:49:02 +0900, Etsuro >>> Fujita<fujita.etsuro@lab.ntt.co.jp> wrote >>> in<5B72C1AE.8010408@lab.ntt.co.jp> >>>> (2018/08/09 22:04), Etsuro Fujita wrote: >>>>> (2018/08/08 17:30), Kyotaro HORIGUCHI wrote: >> >>>> I spent more time looking at the patch. ISTM that the patch well >>>> suppresses the effect of the tuple-descriptor expansion by making >>>> changes to code in the planner and executor (and ruleutils.c), but I'm >>>> still not sure that the patch is the right direction to go in, because >>>> ISTM that expanding the tuple descriptor on the fly might be a wart. >> >>> The exapansion should be safe if the expanded descriptor has the >>> same defitions for base columns and all the extended coulumns are >>> junks. The junk columns should be ignored by unrelated nodes and >>> they are passed safely as far as ForeignModify passes tuples as >>> is from underlying ForeignScan to ForeignUpdate/Delete. >> >> I'm not sure that would be really safe. Does that work well when >> EvalPlanQual, for example? > > Nothing. The reason was that core just doesn't know about the > extended portion. So only problematic case was > ExprEvalWholeRowVar, where explicit sanity check is > perfomed. But, I think it is a ugly wart as you said. So the > latest patch generates full fdw_scan_tlist. Will review. >>>> If we take the Param-based approach suggested by Tom, I suspect there >>>> would be no need to worry about at least those things, so I'll try to >>>> update your patch as such, if there are no objections from you (or >>>> anyone else). >> >>> PARAM_EXEC is single storage side channel that can work as far as >>> it is set and read while each tuple is handled. In this case >>> postgresExecForeignUpdate/Delete must be called before >>> postgresIterateForeignScan returns the next tuple. An apparent >>> failure case for this usage is the join-update case below. >>> >>> https://www.postgresql.org/message-id/20180605.191032.256535589.horiguchi.kyotaro@lab.ntt.co.jp >> >> What I have in mind would be to 1) create a tlist that contains not >> only Vars/PHVs but Params, for each join rel involving the target rel >> so we ensure that the Params will propagate up through all join plan >> steps, and 2) convert a join rel's tlist Params into Vars referencing >> the same Params in the tlists for the outer/inner rels, by setrefs.c. >> I think that would probably work well even for the case you mentioned >> above. Maybe I'm missing something, though. > > As I wrote above, the problem was not param id propagation but > the per-query storage for a parameter holded in econtext. > > PARAM_EXEC is assumed to be used between outer and inner > relations of a nestloop or retrieval from sub-query retrieval as > commented in primnodes.h. > >> PARAM_EXEC: The parameter is an internal executor parameter, used >> for passing values into and out of sub-queries or from >> nestloop joins to their inner scans. >> For historical reasons, such parameters are numbered from 0. >> These numbers are independent of PARAM_EXTERN numbers. Yeah, but IIUC, I think that #2 would allow us to propagate up the param values, not the param ids. > I'm waiting for the patch. OK, but I will review your patch first. Best regards, Etsuro Fujita
pgsql-hackers by date: