From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
To: Alexander Lakhin <exclusion@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Gregory Stark (as CFM) <stark.cfm@gmail.com>
Cc: Michał Kłeczek <michal@kleczek.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Removing unneeded self joins
Date: Thu, 2 May 2024 16:44:54 +0700
Message-ID: <8100af4a-d0aa-450d-a09f-541f26b04ac2@postgrespro.ru> (raw)
In-Reply-To: <CAPpHfdssga9CBQQ6s32t1Xb2Js2WuPfiporbAse334CJiyBb1Q@mail.gmail.com>
References: <64486b0b-0404-e39e-322d-0801154901f3@postgrespro.ru>
<CAPpHfdt9G3EU+vXuqexhweEQoMF-acPUUFuB6hV2Kg0gCy7AjA@mail.gmail.com>
<ebe66358-428c-439f-85dd-da72c9b2b0c2@postgrespro.ru>
<CAPpHfdt-0kVV7O==aJEbjY2iGYBu+XBzTHEbPv_6sVNeC7fffQ@mail.gmail.com>
<48c11b23-9c23-440e-9a3c-020e05196449@postgrespro.ru>
<CAPpHfdsQkY0Gu7bQ9Ekdyzo_5Xxz74TYKbwmk1on1WAASpxXcQ@mail.gmail.com>
<8800ccf2-ae82-461c-92a1-d4c0a22f8cf2@postgrespro.ru>
<CAPpHfdvJ3jBQaJQKrf69K++kXi+Tvzn6TXO-DKXj0KVJjvMebg@mail.gmail.com>
<1bd80083-a13e-4e9f-a7ce-73c3faab521c@postgrespro.ru>
<CAPpHfdsz8E085YF2VoJe2+h4YAShsh7p--t3kQfQTntiCy63Xg@mail.gmail.com>
<922623af-872e-4cdc-9238-129b18bee42d@postgrespro.ru>
<CAPpHfduoAHTp6O7Q8PDe_w=30v0n_06pGe5M65iR4DYcfaYnRw@mail.gmail.com>
<0ed2cf02-e655-6ec7-a4fe-52fd8d572f65@gmail.com>
<CAPpHfdsHWbdPVzUdmoKv3dpo8RHJOa=LQR-ZAT+89uvktMs9XA@mail.gmail.com>
<735b5cd4-568d-fbf7-ad8c-dcfb4c3af86e@gmail.com>
<CAPpHfdssga9CBQQ6s32t1Xb2Js2WuPfiporbAse334CJiyBb1Q@mail.gmail.com>
On 5/1/24 18:59, Alexander Korotkov wrote:
> I think we probably could forbid SJE for the tables with TABLESAMPLE
> altogether. Please, check the attached patch.
Your patch looks good to me. I added some comments and test case into
the join.sql.
One question for me is: Do we anticipate other lateral self-references
except the TABLESAMPLE case? Looking into the extract_lateral_references
implementation, I see the only RTE_SUBQUERY case to be afraid of. But we
pull up subqueries before extracting lateral references. So, if we have
a reference to a subquery, it means we will not flatten this subquery
and don't execute SJE. Do we need more code, as you have written in the
first patch?
--
regards,
Andrei Lepikhov
Postgres Professional
Attachments:0001-Forbid-self-join-elimination-on-table-sampling-scans.patch (text/x-patch, 3.7K) download
From dac8afd2095586921dfcf5564e7f2979e89b77de Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <lepihov@gmail.com>
Date: Thu, 2 May 2024 16:17:52 +0700
Subject: [PATCH] Forbid self-join elimination on table sampling scans.
Having custom table sampling methods we can stuck into unpredictable issues
replacing join with scan operation. It may had sense to analyse possible
situations and enable SJE, but the real profit from this operation looks
too low.
---
src/backend/optimizer/plan/analyzejoins.c | 8 ++++++++
src/backend/optimizer/plan/initsplan.c | 5 ++++-
src/test/regress/expected/join.out | 13 +++++++++++++
src/test/regress/sql/join.sql | 5 +++++
4 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.cindex 506fccd20c..bb89558dcd 100644--- a/src/backend/optimizer/plan/analyzejoins.c+++ b/src/backend/optimizer/plan/analyzejoins.c@@ -2148,6 +2148,14 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids)
Assert(root->simple_rte_array[k]->relid ==
root->simple_rte_array[r]->relid);
+ /*+ * To avoid corner cases with table sampling methods just forbid+ * SJE for table sampling entries.+ */+ if (root->simple_rte_array[k]->tablesample ||+ root->simple_rte_array[r]->tablesample)+ continue;+
/*
* It is impossible to eliminate join of two relations if they
* belong to different rules of order. Otherwise planner can't be
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.cindex e2c68fe6f9..bf839bcaf6 100644--- a/src/backend/optimizer/plan/initsplan.c+++ b/src/backend/optimizer/plan/initsplan.c@@ -415,7 +415,10 @@ extract_lateral_references(PlannerInfo *root, RelOptInfo *brel, Index rtindex)
if (!rte->lateral)
return;
- /* Fetch the appropriate variables */+ /* Fetch the appropriate variables.+ * Changes in this place may need changes in SJE logic, see+ * the remove_self_joins_one_group routine.+ */
if (rte->rtekind == RTE_RELATION)
vars = pull_vars_of_level((Node *) rte->tablesample, 0);
else if (rte->rtekind == RTE_SUBQUERY)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.outindex 8b640c2fc2..63143fe55f 100644--- a/src/test/regress/expected/join.out+++ b/src/test/regress/expected/join.out@@ -6900,6 +6900,19 @@ where s1.x = 1;
Filter: (1 = 1)
(9 rows)
+-- Check that SJE doesn't touch TABLESAMPLE joins+explain (costs off)+SELECT * FROM emp1 t1 NATURAL JOIN LATERAL+ (SELECT * FROM emp1 t2 TABLESAMPLE SYSTEM (t1.code));+ QUERY PLAN +-----------------------------------------------------+ Nested Loop+ -> Seq Scan on emp1 t1+ -> Sample Scan on emp1 t2+ Sampling: system (t1.code)+ Filter: ((t1.id = id) AND (t1.code = code))+(5 rows)+
-- Check that PHVs do not impose any constraints on removing self joins
explain (verbose, costs off)
select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sqlindex c4c6c7b8ba..184fd0876b 100644--- a/src/test/regress/sql/join.sql+++ b/src/test/regress/sql/join.sql@@ -2652,6 +2652,11 @@ select 1 from emp1 t1 left join
on true
where s1.x = 1;
+-- Check that SJE doesn't touch TABLESAMPLE joins+explain (costs off)+SELECT * FROM emp1 t1 NATURAL JOIN LATERAL+ (SELECT * FROM emp1 t2 TABLESAMPLE SYSTEM (t1.code));+
-- Check that PHVs do not impose any constraints on removing self joins
explain (verbose, costs off)
select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
--
2.39.2
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.lepikhov@postgrespro.ru, aekorotkov@gmail.com, exclusion@gmail.com, guofenglinux@gmail.com, stark.cfm@gmail.com, michal@kleczek.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Removing unneeded self joins
In-Reply-To: <8100af4a-d0aa-450d-a09f-541f26b04ac2@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox