|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||Alexey Grishchenko <agrishchenko(at)pivotal(dot)io>|
|Subject:||Re: Endless loop calling PL/Python set returning functions|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Alexey Grishchenko <agrishchenko(at)pivotal(dot)io> writes:
> Any comments on this patch?
I felt that this was more nearly a bug fix than a new feature, so I picked
it up even though it's nominally in the next commitfest not the current
one. I did not like the code too much as it stood: you were not being
paranoid enough about ensuring that the callstack data structure stayed
in sync with the actual control flow. Also, it didn't work for functions
that modify their argument values (cf the committed regression tests);
you have to explicitly save named arguments not only the "args" version,
and you have to do it for SRF suspend/resume not just recursion cases.
But I cleaned all that up and committed it.
> triggers are a bit different - they depend on modifying the global "TD"
> dictionary inside the Python function, and they return only the status
> string. For them, there is no option of modifying the code to avoid global
> input parameters without breaking the backward compatibility with the old
> enduser code.
Yeah. It might be worth the trouble to include triggers in the
save/restore logic, since at least in principle they can be invoked
recursively; but there's not that much practical use for such cases.
I didn't bother with that in the patch as-committed, but if you want
to follow up with an adjustment for it, I'd take a look.
regards, tom lane
|Next Message||Tom Lane||2016-04-05 19:08:13||Re: [PATH] Jsonb, insert a new value into an array at arbitrary position|
|Previous Message||Alvaro Herrera||2016-04-05 18:52:51||Re: Combining Aggregates|