(2018/08/30 20:37), Kyotaro HORIGUCHI wrote:
At Fri, 24 Aug 2018 21:45:35 +0900, Etsuro Fujita<fujita.ets...@lab.ntt.co.jp> wrote
in<5b7ffdef.6020...@lab.ntt.co.jp>
(2018/08/21 11:01), Kyotaro HORIGUCHI wrote:
At Tue, 14 Aug 2018 20:49:02 +0900, Etsuro
Fujita<fujita.ets...@lab.ntt.co.jp> wrote
in<5b72c1ae.8010...@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.kyot...@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
