Re: Removing unneeded self joins

Hi Tom,

Thanks for the update.


On 2/22/19 03:25, Tom Lane wrote:
> * My compiler was bitching about misplaced declarations, so I moved
> some variable declarations accordingly.  I couldn't help noticing
> that many of those wouldn't have been a problem in the first place
> if you were following project style for loops around list_delete_cell
> calls, which usually look more like this:


Changed them to conform to project style. After a couple of segfaults I 
remembered why I wrote them the way I did: you can't use plain 
'continue' with our style of loops and also have to update prev, and I 
can't seem to remember to do that.


> I notice though that there's one unexplained plan change remaining
> in join.out:
>
> @@ -4365,11 +4365,13 @@ explain (costs off)
>   select p.* from
>     (parent p left join child c on (p.k = c.k)) join parent x on p.k = x.k
>     where p.k = 1 and p.k = 2;
> -        QUERY PLAN
> ---------------------------
> +                   QUERY PLAN
> +------------------------------------------------
>    Result
>      One-Time Filter: false
> -(2 rows)
> +   ->  Index Scan using parent_pkey on parent x
> +         Index Cond: (k = 1)
> +(4 rows)
>   
>   -- bug 5255: this is not optimizable by join removal
>   begin;
>
> That sure looks like a bug.  I don't have time to look for the
> cause right now.


David also asked about this before. This is the same plan you'd get for 
'select * from parent where k = 1 and k = 2', and this plan is exposed 
by join removal. So this is not a bug in join removal itself.


> I also noticed that the test results show that when a table
> is successfully optimized away, the remaining reference seems
> to have the alias of the second reference not the first one.
> That seems a little ... weird.  It's just cosmetic of course, but
> why is that?


Can you point me to a query that looks wrong?


> Also, I did notice that you'd stuck a declaration for
> "struct UniqueIndexInfo" into paths.h, which then compelled you
> to include that header in planmain.h.  This seems like poor style;
> I'd have been inclined to put the struct in pathnodes.h instead.


Moved.


> I wonder why you didn't include the Relids into UniqueIndexInfo as well
> ... and maybe make it a proper Node so that unique_for_rels could be
> printed by outfuncs.c.


We also prove and cache the uniqueness for subqueries, for which the 
UniqueIndexInfo is not relevant, that's why it was optional and stored 
in a parallel list. Now I changed it to UniqueRelInfo which always has 
outerrelids and optionally the unique index.


I also fixed a bug with not updating the references in HAVING clause. 
New version is attached.

-- 
Alexander Kuzmenkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company