Re: Killing off removed rels properly

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Richard Guo <guofenglinux@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-02-20T20:48:04Z
Lists: pgsql-hackers
I wrote:
> But while I'm looking at this, 3c569049b seems kind of broken on
> its own terms.  Why is it populating so little of the IndexOptInfo
> for a partitioned index?  I realize that we're not going to directly
> plan anything using such an index, but not populating fields like
> sortopfamily seems like it's at least leaving stuff on the table,
> and at worst making faulty decisions.

I fixed the other issues discussed in this thread, but along the
way I grew even more concerned about 3c569049b, because I discovered
that it's changed plans in more ways than what its commit message
suggests.  For example, given the setup

CREATE TABLE pa_target (tid integer PRIMARY KEY) PARTITION BY LIST (tid);
CREATE TABLE pa_targetp PARTITION OF pa_target DEFAULT;
CREATE TABLE pa_source (sid integer);

then I get this as of commit 3c569049b7^:

# explain select * from pa_source s left join pa_target t on s.sid = t.tid;
                                          QUERY PLAN                                           
-----------------------------------------------------------------------------------------------
 Nested Loop Left Join  (cost=0.15..544.88 rows=32512 width=8)
   ->  Seq Scan on pa_source s  (cost=0.00..35.50 rows=2550 width=4)
   ->  Index Only Scan using pa_targetp_pkey on pa_targetp t  (cost=0.15..0.19 rows=1 width=4)
         Index Cond: (tid = s.sid)
(4 rows)

and this as of 3c569049b7 and later:

# explain select * from pa_source s left join pa_target t on s.sid = t.tid;
                                 QUERY PLAN                                 
----------------------------------------------------------------------------
 Hash Left Join  (cost=67.38..109.58 rows=2550 width=8)
   Hash Cond: (s.sid = t.tid)
   ->  Seq Scan on pa_source s  (cost=0.00..35.50 rows=2550 width=4)
   ->  Hash  (cost=35.50..35.50 rows=2550 width=4)
         ->  Seq Scan on pa_targetp t  (cost=0.00..35.50 rows=2550 width=4)
(5 rows)

Now, I'm not unhappy about that change: it's clearly a win that we now
realize we'll get at most one matching t row for each s row.  What
I'm unhappy about is that this means a partially-populated IndexOptInfo
is being used for rowcount estimation and perhaps other things.
That seems like sheer folly.  Even if it manages to not dump core
from trying to access a missing field, there's a significant risk of
wrong answers, now or in the future.  Why was it done like that?

			regards, tom lane



Commits

  1. Prevent join removal from removing the query's result relation.

  2. Remove gratuitous assumptions about what make_modifytable can see.

  3. When removing a relation from the query, drop its RelOptInfo.

  4. Allow left join removals and unique joins on partitioned tables