Re: Removing unneeded self joins

On 19 October 2018 at 01:47, Alexander Kuzmenkov
<a.kuzmenkov@postgrespro.ru> wrote:
> Here is a version that compiles.

I had a quick read through this and I think its missing about a 1-page
comment section detailing when we can and when we cannot remove these
self joins, and what measures we must take when we do remove them.

Apart from that, I noted the following during my read:

1. I don't think this is the right way to do this. There are other
places where we alter the varnoold. For example:
search_indexed_tlist_for_var(). So you should likely be doing that too
rather than working around it.

@@ -166,10 +166,13 @@ _equalVar(const Var *a, const Var *b)
  COMPARE_SCALAR_FIELD(vartypmod);
  COMPARE_SCALAR_FIELD(varcollid);
  COMPARE_SCALAR_FIELD(varlevelsup);
- COMPARE_SCALAR_FIELD(varnoold);
- COMPARE_SCALAR_FIELD(varoattno);
  COMPARE_LOCATION_FIELD(location);

+ /*
+ * varnoold/varoattno are used only for debugging and may differ even
+ * when the variables are logically the same.
+ */
+

2. Surely the following loop is incorrect:

for (i = toKeep->min_attr; i <= toKeep->max_attr; i++)
{
int attno = i - toKeep->min_attr;
toKeep->attr_needed[attno] = bms_add_members(toKeep->attr_needed[attno],
toRemove->attr_needed[attno]);
}

What if toRemove has a lower min_attr or higher max_attr?

3. "wind" -> "find"

+ * When we wind such a join, we mark one of the participating relation as

4. I think the following shouldn't be happening:

+------------------------------------------------
  Result
    One-Time Filter: false
-(2 rows)
+   ->  Index Scan using parent_pkey on parent x
+         Index Cond: (k = 1)
+(4 rows)

5. I'd have thought the opposite. Surely there are more chances of
this being useful with more joins?

+ /* Limit the number of joins we process to control the quadratic behavior. */
+ if (n > join_collapse_limit)
+ break;

6. In remove_self_joins_one_level() I think you should collect the
removed relations in a Relids rather than a list.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services