pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Vik Fearing <vik@postgresfriends.org>
Cc: zuming.jiang@inf.ethz.ch, pgsql-bugs@lists.postgresql.org, Alexander Korotkov <akorotkov@postgresql.org>
Subject: Re: BUG #18170: Unexpected error: no relation entry for relid 3
Date: Fri, 27 Oct 2023 18:00:28 +0700
Message-ID: <2eb87c2e-7e6f-4002-8df3-8fac3aa6a037@postgrespro.ru> (raw)
In-Reply-To: <721542ce-79df-469e-93e9-811fadb5bd0c@postgrespro.ru>
References: <18170-f1d17bf9a0d58b24@postgresql.org>
	<4ecab120-6be3-4cf6-a2a0-249c73ae91d4@postgresfriends.org>
	<1189851.1698340331@sss.pgh.pa.us>
	<011bcae5-7840-42c7-a50c-b406189e2e78@postgrespro.ru>
	<CAPpHfdsiN0norSHfw3Ujemf=RuPzMxt5PRDiyv0vqUdwSBDtoA@mail.gmail.com>
	<721542ce-79df-469e-93e9-811fadb5bd0c@postgrespro.ru>

On 27/10/2023 16:07, Andrei Lepikhov wrote:
> On 27/10/2023 15:17, Alexander Korotkov wrote:
>> On Fri, Oct 27, 2023 at 9:31 AM Andrei Lepikhov
>> <a.lepikhov@postgrespro.ru> wrote:
>>> On 27/10/2023 00:12, Tom Lane wrote:
>>>> Vik Fearing <vik@postgresfriends.org> writes:
>>>>> On 10/26/23 16:01, PG Bug reporting form wrote:
>>>>>> My fuzzer finds a bug in Postgres, which triggers an unexpected 
>>>>>> error.
>>>>
>>>>> This bisects to d3d55ce571369dad6e1d582f1655e5a3fbd8594a, Remove 
>>>>> useless
>>>>> self-joins.
>>>>
>>>> I wonder if that new code thinks it can remove ref_2 from the query,
>>>> even though ref_2 is used in the targetlist.  I'm not seeing
>>>> control reach remove_leftjoinrel_from_query, though.
>>>
>>> As I see, this join can be removed: in the explain you can see that
>>> OUTER JOIN is replaced with the INNER JOIN(ref_2, ref_3) ON a key 
>>> column.
>>> In my opinion, the origin of the problem is that the parent_root
>>> contains a link to ref_2 in its
>>> simple_rte_array[]->subquery->targetList. I am still looking for a
>>> general solution right now, but it doesn't look too complicated at first
>>> sight.
>>
>> Yes, I came to the same conclusion.  We process root->parse.  But I
>> didn't get why parent_root->simple_rte_array[]->subquery is not the
>> same as root->parse.  They look the same, but they are distinct
>> copies.  If they were the same pointers, there would be no problem.
> 
> As I see, the copy of the parse tree is induced by the same feature as 
> usual in the last few months: 2489d76. It introduced 
> remove_nulling_relids, and it altered our parse tree. Right now, I don't 
> have an answer: it should be fixed in SJE, or this is a more general 
> issue just discovered by the SJE.

So, I can propose two options. First - don't clean only the current root 
structure, but also make cleanup of the parent. Although it looks safe, 
I am not happy with this approach - it seems too simple: we should have 
a genuine reason for such a cleaning because it potentially adds overhead.
The second option is to add a flag for not altering queries in 
remove_nulling_relids() - it looks like a mistake when we have two 
different query trees in the root and its parent. Also, it reduces 
memory usage a bit.
So, if my analysis is correct, it is better to use the second way (see 
attachment).

-- 
regards,
Andrei Lepikhov
Postgres Professional

Attachments:

0001-Don-t-alter-parse-tree-during-the-procedure-of-outer.patch (text/plain, 3.5K) download From d2fac8ced47e883c7b809a4dc3612d35987f460c Mon Sep 17 00:00:00 2001 From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru> Date: Fri, 27 Oct 2023 17:55:04 +0700 Subject: [PATCH] Don't alter parse tree during the procedure of outer join reduction. --- src/backend/optimizer/prep/prepjointree.c | 16 ++++++++-------- src/backend/rewrite/rewriteManip.c | 10 +++++++++- src/include/rewrite/rewriteManip.h | 5 ++++- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 73ff40721c..06a7636403 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -2687,14 +2687,14 @@ reduce_outer_joins(PlannerInfo *root) if (!bms_is_empty(state2.inner_reduced)) { root->parse = (Query *) - remove_nulling_relids((Node *) root->parse, + remove_nulling_relids_ext((Node *) root->parse, state2.inner_reduced, - NULL); + NULL, QTW_DONT_COPY_QUERY); /* There could be references in the append_rel_list, too */ root->append_rel_list = (List *) - remove_nulling_relids((Node *) root->append_rel_list, + remove_nulling_relids_ext((Node *) root->append_rel_list, state2.inner_reduced, - NULL); + NULL, QTW_DONT_COPY_QUERY); } /* @@ -2707,13 +2707,13 @@ reduce_outer_joins(PlannerInfo *root) Relids full_join_relids = bms_make_singleton(statep->full_join_rti); root->parse = (Query *) - remove_nulling_relids((Node *) root->parse, + remove_nulling_relids_ext((Node *) root->parse, full_join_relids, - statep->unreduced_side); + statep->unreduced_side, QTW_DONT_COPY_QUERY); root->append_rel_list = (List *) - remove_nulling_relids((Node *) root->append_rel_list, + remove_nulling_relids_ext((Node *) root->append_rel_list, full_join_relids, - statep->unreduced_side); + statep->unreduced_side, QTW_DONT_COPY_QUERY); } } diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 32bd2f1dc9..1fdfb05cfb 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -1226,6 +1226,14 @@ Node * remove_nulling_relids(Node *node, const Bitmapset *removable_relids, const Bitmapset *except_relids) +{ + return remove_nulling_relids_ext(node, removable_relids, except_relids, 0); + +} +Node * +remove_nulling_relids_ext(Node *node, + const Bitmapset *removable_relids, + const Bitmapset *except_relids, int flags) { remove_nulling_relids_context context; @@ -1235,7 +1243,7 @@ remove_nulling_relids(Node *node, return query_or_expression_tree_mutator(node, remove_nulling_relids_mutator, &context, - 0); + flags); } static Node * diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h index ca12780bc7..6bc7e6f36e 100644 --- a/src/include/rewrite/rewriteManip.h +++ b/src/include/rewrite/rewriteManip.h @@ -71,7 +71,10 @@ extern Node *add_nulling_relids(Node *node, extern Node *remove_nulling_relids(Node *node, const Bitmapset *removable_relids, const Bitmapset *except_relids); - +extern Node *remove_nulling_relids_ext(Node *node, + const Bitmapset *removable_relids, + const Bitmapset *except_relids, + int flags); extern Node *replace_rte_variables(Node *node, int target_varno, int sublevels_up, replace_rte_variables_callback callback, -- 2.42.0

view thread (34+ messages)  latest in thread

Message-ID: <2eb87c2e-7e6f-4002-8df3-8fac3aa6a037@postgrespro.ru>
Permalink:  ../2eb87c2e-7e6f-4002-8df3-8fac3aa6a037@postgrespro.ru/
Also on:    postgresql.org/message-id/2eb87c2e-7e6f-4002-8df3-8fac3aa6a037@postgrespro.ru

 ·  · 

reply

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-bugs@postgresql.org
  Cc: a.lepikhov@postgrespro.ru, aekorotkov@gmail.com, tgl@sss.pgh.pa.us, vik@postgresfriends.org, akorotkov@postgresql.org
  Subject: Re: BUG #18170: Unexpected error: no relation entry for relid 3
  In-Reply-To: <2eb87c2e-7e6f-4002-8df3-8fac3aa6a037@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