Re: Removing unneeded self joins

Alena Rybakina <lena.ribackina@yandex.ru>

From: Alena Rybakina <lena.ribackina@yandex.ru>
To: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Cc: "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, Michał Kłeczek <michal@kleczek.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Alexander Korotkov <aekorotkov@gmail.com>
Date: 2023-10-10T19:29:58Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Remove GUC_NOT_IN_SAMPLE from enable_self_join_elimination

  2. Put enable_self_join_elimination into postgresql.conf.sample

  3. Get rid of ojrelid local variable in remove_rel_from_query()

  4. Implement Self-Join Elimination

  5. Revert: Remove useless self-joins

  6. Replace lateral references to removed rels in subqueries

  7. Replace relids in lateral subquery parse tree during SJE

  8. Forbid SJE with result relation

  9. Fix misuse of RelOptInfo.unique_for_rels cache by SJE

  10. Replace the relid in some missing fields during SJE

  11. Revert 56-bit relfilenode change and follow-up commits.

  12. Stabilize timetz test across DST transitions.

  13. Speed up finding EquivalenceClasses for a given set of rels

  14. Fix mark-and-restore-skipping test case to not be a self-join.

Attachments

Hi!

I have reviewed your patch and I noticed a few things.

First of all, I think I found a bug in your latest patch version, and 
this query shows it:

EXPLAIN (COSTS OFF)
SELECT c.oid, e.oid FROM pg_class c FULL JOIN (
   SELECT e1.oid FROM pg_extension e1, pg_extension e2
   WHERE e1.oid=e2.oid) AS e
   ON c.oid=e.oid;

In the current version we get such a query plan:

                QUERY PLAN
-----------------------------------------
  Hash Full Join
    Hash Cond: (c.oid = e2.oid)
    ->  Seq Scan on pg_class c
    ->  Hash
          ->  Seq Scan on pg_extension e2
(5 rows)

But I think it should be:

QUERY PLAN
-----------------------------------------
Hash Full Join
Hash Cond: (c.oid = e2.oid)
-> Seq Scan on pg_class c
-> Hash
-> Seq Scan on pg_extension e2
*Filter: (oid IS NOT NULL)*
(6 rows)

I have looked at the latest version of the code, I assume that the error 
lies in the replace_varno_walker function, especially in the place where 
we check the node by type Var, and does not form any NullTest node.

if (OidIsValid(reloid) && get_attnotnull(reloid, attnum)) -- this 
condition works
         {
           rinfo->clause = (Expr *) makeBoolConst(true, false);
         }
         else
         {
           NullTest   *ntest = makeNode(NullTest);

           ntest->arg = leftOp;
           ntest->nulltesttype = IS_NOT_NULL;
           ntest->argisrow = false;
           ntest->location = -1;
           rinfo->clause = (Expr *) ntest;
         }


Secondly, I added some code in some places to catch erroneous cases and 
added a condition when we should not try to apply the self-join-removal 
transformation due to the absence of an empty self-join list after 
searching for it and in general if there are no joins in the query. 
Besides, I added a query for testing and wrote about it above. I have 
attached my diff file.


In addition, I found a comment for myself that was not clear to me. I 
would be glad if you could explain it to me.

You mentioned superior outer join in the comment, unfortunately, I 
didn't find anything about it in the PostgreSQL code, and this 
explanation remained unclear to me. Could you explain in more detail 
what you meant?

/*
  * At this stage joininfo lists of inner and outer can contain
  * only clauses, required for *a superior outer join* that can't
  * influence on this optimization. So, we can avoid to call the
  * build_joinrel_restrictlist() routine.
*/
  restrictlist = generate_join_implied_equalities(root, joinrelids,
                               inner->relids,
                               outer, NULL);

-- 

Regards,
Alena Rybakina