Re: Procedures versus the "fastpath" API

On Wed, Mar 10, 2021 at 10:03:24AM +0100, Laurenz Albe wrote:
> On Tue, 2021-03-09 at 14:15 -0500, Tom Lane wrote:
> > As for procedures, I'm of the opinion that we should just reject those
> > too, but some other security team members were not happy with that
> > idea.  Conceivably we could attempt to make the case actually work,
> > but is it worth the trouble?  Given the lack of field complaints
> > about the "invalid transaction termination" failure, it seems unlikely
> > that it's occurred to anyone to try to call procedures this way.
> > We'd need special infrastructure to test the case, too, since psql
> > offers no access to fastpath calls.
> > 
> > A compromise suggestion was to prohibit calling procedures via
> > fastpath as of HEAD, but leave existing releases as they are,
> > in case anyone is using a case that happens to work.

(That was my suggestion.)

> The "invalid transaction termination" failure alone doesn't
> worry or surprise me - transaction handling in procedures only works
> under rather narrow conditions anyway (no SELECT on the call stack,
> no explicit transaction was started outside the procedure).
> 
> If that is the only problem, I'd just document it.  The hard work is
> of course that there is no other problem with calling procedures that
> way.  If anybody wants to do that work, and transaction handling is
> the only thing that doesn't work with the fastpath API, we can call
> it supported and document the exception.

I'd be fine with that, too.

> In case of doubt, I would agree with you and forbid it in HEAD
> as a corner case with little real-world use.

The PQfn(some-procedure) feature has no known bugs and no known users, so I
think the decision carries little weight.  Removing the feature would look
wise if we later discover some hard-to-fix bug therein.  Removal also obeys
the typical pattern that a given parse context accepts either procedures or
functions, not both.  Keeping the feature, at least in back branches, would
look wise if it avoids urgent s/PQfn/PQexecParams/ work for some user trying
to update to 13.3.