On Sat, Jan 30, 2016 at 07:37:45AM -0500, Robert Haas wrote:
> On Fri, Jan 29, 2016 at 11:19 PM, Noah Misch <noah@leadboat.com> wrote:
> > As you say, forbidding things makes friction in the event that someone comes
> > along wanting to do the forbidden thing. Forbidding things also simplifies
> > the system, making it easier to verify. This decision should depend on the
> > API's audience. We have prominently-public APIs like lsyscache.h,
> > stringinfo.h and funcapi.h. They should cater to reasonably-foreseeable use
> > cases, not just what yesterday's users have actually used. We then have
> > narrow-interest APIs like subselect.h and view.h. For those, the value of a
> > simpler system exceeds the risk-adjusted cost of friction. They should impose
> > strict constraints on their callers.
> >
> > Portal status belongs to the second category. PortalRun(), PortalRunFetch()
> > and PersistHoldablePortal() are the three core functions that place portals
> > into PORTAL_ACTIVE status. No PGXN module does so; none so much as references
> > a PortalStatus constant or MarkPortal{Active,Done,Failed}() function. If
> > someone adds a fourth core portal runner, the system will be simpler and
> > better if that function replicates the structure of the existing three.
>
> I won't argue with that, but it strikes me that if I were reviewing a
> patch to do such a thing, I'd surely compare the new caller against
> the existing ones, so any failure to adhere to the same design pattern
> would be caught that way. I expect other reviewers and/or committers
> would almost certainly do something similar. If we presuppose
> incompetence on the part of future reviewers and committers, no action
> taken now can secure us.
You could offer that paragraph as an objection to almost all Assert(), elog(),
and automated tests. Why levy it against this patch? The valuable ways
assertions and tests supplement review are well-established.
> You may be right, but then again Tom had a different opinion, even
> after seeing your patch, and he's no dummy.
Eh? Tom last posted to this thread before I first posted a patch.
