On 2017-07-26 16:28:38 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2017-07-26 15:03:37 -0400, Tom Lane wrote:
> >> Hm, that seems kinda backwards to me; I was envisioning the checks
> >> moving to the callees not the callers. I think it'd end up being
> >> about the same number of occurrences of CHECK_FOR_INTERRUPTS(),
> >> and there would be less of a judgment call about where to put them.
>
> > Hm, that seems a bit riskier - easy to forget one of the places where we
> > might need a CFI().
>
> I would argue the contrary. If we put a CFI at the head of each node
> execution function, then it's just boilerplate that you copy-and-paste
> when you invent a new node type. The way you've coded it here, it
> seems to involve a lot of judgment calls. That's very far from being
> copy and paste, and the more different it looks from one node type
> to another, the easier it will be to forget it.
>
> > We certainly are missing a bunch of them in various nodes
>
> It's certainly possible that there are long-running loops not involving
> any ExecProcNode recursion at all, but that would be a bug independent
> of this issue. The CFI in ExecProcNode itself can be replaced exactly
> either by asking all callers to do it, or by asking all callees to do it.
> I think the latter is going to be more uniform and harder to screw up.
Looks a bit better. Still a lot of judgement-y calls tho, e.g. when one
node function just calls the next, or when there's loops etc. I found
a good number of missing CFIs...
What do you think?
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
