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
