From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Get rid of runtime handling of AlternativeSubPlan?
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1992951.1592785225.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 21 Jun 2020 20:20:25 -0400
Message-ID: <1992952.1592785225@sss.pgh.pa.us>
Precedence: bulk

Back in bd3daddaf232d95b0c9ba6f99b0170a0147dd8af, which introduced
AlternativeSubPlans, I wrote:
    =

  There is a lot more that could be done based on this infrastructure: in
  particular it's interesting to consider switching to the hash plan if we=
 start
  out using the non-hashed plan but find a lot more upper rows going by th=
an we
  expected.  I have therefore left some minor inefficiencies in place, suc=
h as
  initializing both subplans even though we will currently only use one.

That commit will be twelve years old come August, and nobody has either
built anything else atop it or shown any interest in making the plan choic=
e
switchable mid-run.  So it seems like kind of a failed experiment.

Therefore, I'm considering the idea of ripping out all executor support
for AlternativeSubPlan and instead having the planner replace an
AlternativeSubPlan with the desired specific SubPlan somewhere late in
planning, possibly setrefs.c.

Admittedly, the relevant executor support only amounts to a couple hundred
lines, but that's not nothing.  A perhaps-more-useful effect is to get rid
of the confusing and poorly documented EXPLAIN output that you get for an
AlternativeSubPlan.

I also noted that the existing subplan-selection logic in
ExecInitAlternativeSubPlan is really pretty darn bogus, in that it uses a
one-size-fits-all execution count estimate of parent->plan->plan_rows, no
matter which subexpression the subplan is in.  This is only appropriate
for subplans in the plan node's targetlist, and can be either too high
or too low elsewhere.  It'd be relatively easy for setrefs.c to do
better, I think, since it knows which subexpression it's working on
at any point.

Thoughts?

			regards, tom lane