Horton

Re: Removing unneeded self joins

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Andrei Lepikhov <a.lepikhov@postgrespro.ru>, Richard Guo <guofenglinux@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, Michał Kłeczek <michal@kleczek.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-12-09T06:03:10Z
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.

On Sat, Jul 20, 2024 at 2:38 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> We initially didn't use ChangeVarNodes() in SJE at all.  See the last
> patch version without it [1].  We're trying to address Tom Lane's
> proposal to re-use more of existing tree-manipulation infrastructure
> [2].  I agree with you that the case with ChangeVarNodes() looks
> questionable.  Do you have other ideas how we can re-use some more of
> existing tree-manipulation infrastructure in SJE?

I think there was one idea to be considered.  Given that this feature
has fragility to changes in PlannerInfo and underlying structures,
Andrei proposed to generate a walker over PlannerInfo with a Perl
script.  I spent some time analyzing this idea.  I came to the
following conclusions.

It seems feasible to generate a function that would walk over all
PlannerInfo fields recursively.  Perhaps some more meta-information is
needed, at least, to check whether we should visit a particular
pointer field.  But that could be provided by additional
pg_node_attr().  With this approach, we would need to sacrifice some
efficiency (e.g., processing all the EquivalenceClasses instead of
picking only the ones used in the relation to be removed).  Also, the
logic in remove_self_join_rel() is more complex than processing all
the Relids.  That is, we would still need a massive number of
if-branches specifying how we handle each node type.  It doesn't look
like we would end up with a more simple or less error-prone
implementation.

We may decide to generate not just a walker but most of the logic in
remove_self_join_rel().  This is probably possible by injecting way
more meta-information into definitions of structures.  That isn't
going to be simpler than the current approach.  But it is probably
less error-prone: one could realize that if you add a new field to the
structure, it should have a similar pg_node_attr() as surroundings.
But I am afraid that if we go this way, we may end up with an awkward
heap of pg_node_attr() to generate the functionality of
remove_self_join_rel().  Should we better add comments to PlannerInfo
and other relevant structures saying: if you're going to modify this,
consider how that affects remove_self_join_rel()?

Any thoughts?

Links
1. https://www.postgresql.org/message-id/flat/64486b0b-0404-e39e-322d-0801154901f3%40postgrespro.ru

------
Regards,
Alexander Korotkov
Supabase