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.
