On 13.10.2023 12:03, Andrei Lepikhov wrote:
> 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.
>
Yes, sorry, I missed it. thanks again for the explanation 🙂