On 31/10/2020 11:26, Andrey V. Lepikhov wrote:
+ /*
+ * Process restrictlist to seperate out the self join
quals from
+ * the other quals. e.g x = x goes to selfjoinquals and
a = b to
+ * otherjoinquals.
+ */
+ split_selfjoin_quals(root, restrictlist, &selfjoinquals,
+
&otherjoinquals);
+
+ if (list_length(selfjoinquals) == 0)
+ {
+ /*
+ * Have a chance to remove join if target list
contains vars from
+ * the only one relation.
+ */
+ if (list_length(otherjoinquals) == 0)
+ {
+ /* Can't determine uniqueness without
any quals. */
+ continue;
+
+ }
+ else if (!tlist_contains_rel_exprs(root,
joinrelids, inner))
+ {
+ if (!innerrel_is_unique(root, joinrelids,
outer->relids,
+
inner, JOIN_INNER, otherjoinquals,
+
false))
+ continue;
+ }
+ else
+ /*
+ * The target list contains vars from
both inner and outer
+ * relations.
+ */
+ continue;
+ }
+
+ /*
+ * Determine if the inner table can duplicate outer
rows. We must
+ * bypass the unique rel cache here since we're
possibly using a
+ * subset of join quals. We can use 'force_cache' =
true when all
+ * join quals are selfjoin quals. Otherwise we could
end up
+ * putting false negatives in the cache.
+ */
+ else if (!innerrel_is_unique(root, joinrelids,
outer->relids,
+
inner, JOIN_INNER, selfjoinquals,
+
list_length(otherjoinquals) == 0))
+ continue;
I don't understand the logic here. If 'selfjoinquals' is empty, it means
that there is no join qual between the two relations, right? How can we
ever remove the join in that case? And how does the target list affect
that? Can you give an example query of that?
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4553,11 +4553,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 doesn't seem like an improvement...
My general impression of this patch is that it's a lot of code to deal
with a rare special case. At the beginning of this thread there was
discussion on the overhead that this might add to planning queries that
don't benefit, but adding a lot of code isn't nice either, even if the
performance is acceptable. That's not necessarily a show-stopper, but it
does make me much less excited about this. I'm not sure what to suggest
to do about that, except a really vague "Can you make is simpler?"
- Heikki
