Re: Propagate pathkeys from CTEs up to the outer query

Richard Guo <guofenglinux@gmail.com> writes:
> I agree with your points.  Previously I was thinking that CTEs were the
> only scenario where we needed to remember the best path and only
> required the best path's pathkeys.  However, considering potential
> future use cases as you mentioned, I concur that having a per-subplan
> list of paths would be more future-proof.  Please see attached v4 patch.

Hm, well, you didn't actually fill in the paths for the other
subqueries.  I agree that it's not worth doing so in
SS_make_initplan_from_plan, but a comment explaining that decision
seems in order.  Also, there's nothing stopping us from saving the
path for subplans made in build_subplan, except adding a parameter
to pass them down.  So I did that, and made a couple other cosmetic
changes, and pushed it.

One thing I noticed while testing is that while your regression-test
query successfully propagates the pathkeys:

regression=# explain with x as materialized (select unique1 from tenk1 b order by unique1)
select count(*) from tenk1 a
  where unique1 in (select * from x);
                                             QUERY PLAN
----------------------------------------------------------------------------------------------------
 Aggregate  (cost=915.55..915.56 rows=1 width=8)
   CTE x
     ->  Index Only Scan using tenk1_unique1 on tenk1 b  (cost=0.29..270.29 rows=10000 width=4)
   ->  Merge Semi Join  (cost=0.31..620.26 rows=10000 width=0)
         Merge Cond: (a.unique1 = x.unique1)
         ->  Index Only Scan using tenk1_unique1 on tenk1 a  (cost=0.29..270.29 rows=10000 width=4)
         ->  CTE Scan on x  (cost=0.00..200.00 rows=10000 width=4)
(7 rows)

this variant doesn't:

regression=# explain with x as materialized (select unique1 from tenk1 b)
select count(*) from tenk1 a
  where unique1 in (select * from x);
                                             QUERY PLAN
----------------------------------------------------------------------------------------------------
 Aggregate  (cost=1028.07..1028.08 rows=1 width=8)
   CTE x
     ->  Index Only Scan using tenk1_unique1 on tenk1 b  (cost=0.29..270.29 rows=10000 width=4)
   ->  Hash Semi Join  (cost=325.28..732.78 rows=10000 width=0)
         Hash Cond: (a.unique1 = x.unique1)
         ->  Index Only Scan using tenk1_unique1 on tenk1 a  (cost=0.29..270.29 rows=10000 width=4)
         ->  Hash  (cost=200.00..200.00 rows=10000 width=4)
               ->  CTE Scan on x  (cost=0.00..200.00 rows=10000 width=4)
(8 rows)

That's not the fault of anything we did here; the IndexOnlyScan path
in the subquery is in fact not marked with any pathkeys, even though
clearly its result is sorted.  I believe that's an intentional
decision from way way back, that pathkeys only correspond to orderings
that are of interest in the current query level.  "select unique1 from
tenk1 b order by unique1" has an interest in ordering by unique1,
but "select unique1 from tenk1 b" does not, so it's choosing that
path strictly according to cost.  Not generating pathkeys in such a
query saves a few cycles and ensures that we won't improperly prefer
a path on the basis of pathkeys if it hasn't got a cost advantage.
So I'm quite hesitant to muck with that old decision, especially in
the waning days of a development cycle, but the results do feel a
little strange here.

            regards, tom lane