Re: Removing unneeded self joins

On 13/10/2023 15:56, a.rybakina wrote:
>
>>> Also I've incorporated improvements from Alena Rybakina except one for
>>> skipping SJ removal when no SJ quals is found.  It's not yet clear for
>>> me if this check fix some cases. But at least optimization got skipped
>>> in some useful cases (as you can see in regression tests).
>>
>> Agree. I wouldn't say I like it too. But also, I suggest skipping some
>> unnecessary assertions proposed in that patch:
>> Assert(toKeep->relid != -1); - quite strange. Why -1? Why not all the
>> negative numbers, at least?
>> Assert(is_opclause(orinfo->clause)); - above we skip clauses with
>> rinfo->mergeopfamilies == NIL. Each mergejoinable clause is already
>> checked as is_opclause.
>> All these changes (see in the attachment) are optional.
>>
> I don't mind about asserts, maybe I misunderstood something in the patch.
>
> About skipping SJ removal when no SJ quals is found, I assume it is
> about it:
>
> split_selfjoin_quals(root, restrictlist, &selfjoinquals,
>                                   &otherjoinquals, inner->relid,
> outer->relid);
>
> +            if (list_length(selfjoinquals) == 0)
> +             {
> +                 /*
> +                  * XXX:
> +                  * we would detect self-join without quals like 'x==x'
> if we had
> +                  * an foreign key constraint on some of other quals
> and this join
> +                  * haven't any columns from the outer in the target list.
> +                  * But it is still complex task.
> +                  */
> +                 continue;
> +             }
>
> as far as I remember, this is the place where it is checked that the SJ
> list is empty and it is logical, in my opinion, that no transformations
> should be performed if no elements are found for them.
You forget we have "Degenerate" case, as Alexander mentioned above. What
if you have something like that:
SELECT ... FROM A a1, A a2 WHERE a1.id=1 AND a2.id=1;
In this case, uniqueness can be achieved by the baserestrictinfo
"A.id=1", if we have an unique index on this column.

--
regards,
Andrey Lepikhov
Postgres Professional