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 🙂
