From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Get rid of runtime handling of AlternativeSubPlan?
In-reply-to: 
 <CAApHDvqhcEab0aJ29C+yznHiUHNqoKvTOL+sz0p3a1SNon4ntg@mail.gmail.com>
References: <1992952.1592785225@sss.pgh.pa.us>
 <3740825.1598743596@sss.pgh.pa.us>
 <CAApHDvqhcEab0aJ29C+yznHiUHNqoKvTOL+sz0p3a1SNon4ntg@mail.gmail.com>
Comments: In-reply-to David Rowley <dgrowleyml@gmail.com>
	message dated "Mon, 31 Aug 2020 14:35:32 +1200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3968495.1598888506.1@sss.pgh.pa.us>
Date: Mon, 31 Aug 2020 11:41:46 -0400
Message-ID: <3968496.1598888506@sss.pgh.pa.us>
Precedence: bulk

David Rowley <dgrowleyml@gmail.com> writes:
> Do you feel that the choice to create_plan() on the subplan before
> planning the outer query is still a good one?  ISTM that that was
> required when the AlternativeSubplan decision was made during
> execution, since we, of course, need a plan to execute. If the
> decision is now being made in the planner then is it not better to
> delay the create_plan() until later in planning?

Hm.  That's well outside the scope I had in mind for this patch.
In principle, you're right that we could postpone final planning
of the subquery till later; but I fear it'd require quite a lot
of refactoring to make it work that way.  There's a lot of rather
subtle timing dependencies in the processing done by createplan.c
and setrefs.c, so I think this might be a lot more painful than
it seems at first glance.  And we'd only gain anything in cases that
use AlternativeSubPlan, which is a minority of subplans, so on the
whole I rather doubt it's worth the trouble.

One inefficiency I see that we could probably get rid of is
where make_subplan() is doing

            /* Now we can check if it'll fit in hash_mem */
            /* XXX can we check this at the Path stage? */
            if (subplan_is_hashable(plan))
            {

The only inputs subplan_is_hashable needs are the predicted rowcount
and output width, which surely we could get from the Path.  So we
could save doing create_plan() when we decide the subquery output is
too big to hash.  OTOH, that's probably a pretty small minority of
use-cases, so it might not be worth troubling over.

			regards, tom lane