From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Richard Guo <guofenglinux@gmail.com>
Cc: 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: Sat, 28 Oct 2023 13:59:23 +0700
Message-ID: <c0a5b39d-8a80-47fb-bd34-6d769f88df8f@postgrespro.ru> (raw)
In-Reply-To: <CAMbWs494m3AjhHry_HFCFi_SJ+6-PaeMRbx7jK39ZHmK0MVOUA@mail.gmail.com>
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>
<2eb87c2e-7e6f-4002-8df3-8fac3aa6a037@postgrespro.ru>
<CAMbWs494m3AjhHry_HFCFi_SJ+6-PaeMRbx7jK39ZHmK0MVOUA@mail.gmail.com>
On 27/10/2023 21:10, Richard Guo wrote:
>
> On Fri, Oct 27, 2023 at 7:00 PM Andrei Lepikhov
> <a.lepikhov@postgrespro.ru <mailto:a.lepikhov@postgrespro.ru>> wrote:
>
> 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).
>
>
> Alternatively, can we look at subroot->parse->targetList instead of
> subquery->targetList where we call estimate_num_groups on the output of
> the subquery?
It is a solution. But does it mask the real problem? In my mind, we copy
node trees to use somewhere else or probe a conjecture. Here, we have
two different representations of the same subquery. Keeping aside the
memory consumption issue, is it correct?
Make sense to apply both options: switch the groups estimation to
subroot targetList and keep one version of a subquery.
In attachment - second (combined) version of the change. Here I added
assertions to check identity of root->parse and incoming query tree.
>
> --- a/src/backend/optimizer/prep/prepunion.c
> +++ b/src/backend/optimizer/prep/prepunion.c
> @@ -341,7 +341,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
> *pNumGroups = subpath->rows;
> else
> *pNumGroups = estimate_num_groups(subroot,
> -
> get_tlist_exprs(subquery->targetList, false),
> +
> get_tlist_exprs(subroot->parse->targetList, false),
> subpath->rows,
> NULL,
> NULL);
>
> BTW, I'm a little surprised that QTW_DONT_COPY_QUERY doesn't seem to be
> used anywhere currently.
I too. But we use this flag in the enterprise fork to reduce memory
consumption. It could be proposed for upstream, but looks a bit unsafe.
I guess, some extensions could do the same.
--
regards,
Andrei Lepikhov
Postgres Professional
Attachments:v2-0001-Don-t-alter-parse-tree-during-the-procedure-of-outer.patch (text/plain, 7.7K) download
From 5992cc20fabed21bd493268ae3fb96b4891adadd 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/path/allpaths.c | 3 +++
src/backend/optimizer/plan/planner.c | 3 +++
src/backend/optimizer/plan/subselect.c | 9 +++++++++
src/backend/optimizer/prep/prepjointree.c | 24 +++++++++++------------
src/backend/optimizer/prep/prepunion.c | 5 ++++-
src/backend/rewrite/rewriteManip.c | 10 +++++++++-
src/include/rewrite/rewriteManip.h | 5 ++++-
7 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.cindex 67921a0826..e8c57b4e2e 100644--- a/src/backend/optimizer/path/allpaths.c+++ b/src/backend/optimizer/path/allpaths.c@@ -2652,6 +2652,9 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
root,
false, tuple_fraction);
+ /* Keep only one copy of the subquery */+ Assert(subquery == rel->subroot->parse);+
/* Isolate the params needed by this specific subplan */
rel->subplan_params = root->plan_params;
root->plan_params = NIL;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.cindex a8cea5efe1..501688c012 100644--- a/src/backend/optimizer/plan/planner.c+++ b/src/backend/optimizer/plan/planner.c@@ -413,6 +413,9 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
root = subquery_planner(glob, parse, NULL,
false, tuple_fraction);
+ /* Keep only one copy of the subquery */+ Assert(parse == root->parse);+
/* Select best Path and turn it into a Plan */
final_rel = fetch_upper_rel(root, UPPERREL_FINAL, NULL);
best_path = get_cheapest_fractional_path(final_rel, tuple_fraction);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.cindex 7a9fe88fec..92ac5f0931 100644--- a/src/backend/optimizer/plan/subselect.c+++ b/src/backend/optimizer/plan/subselect.c@@ -222,6 +222,9 @@ make_subplan(PlannerInfo *root, Query *orig_subquery,
root,
false, tuple_fraction);
+ /* Keep only one copy of the subquery */+ Assert(subquery == subroot->parse);+
/* Isolate the params needed by this specific subplan */
plan_params = root->plan_params;
root->plan_params = NIL;
@@ -269,6 +272,9 @@ make_subplan(PlannerInfo *root, Query *orig_subquery,
root,
false, 0.0);
+ /* Keep only one copy of the subquery */+ Assert(subquery == subroot->parse);+
/* Isolate the params needed by this specific subplan */
plan_params = root->plan_params;
root->plan_params = NIL;
@@ -982,6 +988,9 @@ SS_process_ctes(PlannerInfo *root)
root,
cte->cterecursive, 0.0);
+ /* Keep only one copy of the subquery */+ Assert(subquery == subroot->parse);+
/*
* Since the current query level doesn't yet contain any RTEs, it
* should not be possible for the CTE to have requested parameters of
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.cindex 73ff40721c..db0668f61f 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);
}
}
@@ -3172,14 +3172,14 @@ remove_useless_result_rtes(PlannerInfo *root)
if (!bms_is_empty(dropped_outer_joins))
{
root->parse = (Query *)
- remove_nulling_relids((Node *) root->parse,+ remove_nulling_relids_ext((Node *) root->parse,
dropped_outer_joins,
- 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,
dropped_outer_joins,
- NULL);+ NULL, QTW_DONT_COPY_QUERY);
}
/*
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.cindex 0c68ec011b..fbe39e20fd 100644--- a/src/backend/optimizer/prep/prepunion.c+++ b/src/backend/optimizer/prep/prepunion.c@@ -243,6 +243,9 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
false,
root->tuple_fraction);
+ /* Keep only one copy of the subquery */+ Assert(rte->subquery == subroot->parse);+
/*
* It should not be possible for the primitive query to contain any
* cross-references to other primitive queries in the setop tree.
@@ -341,7 +344,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
*pNumGroups = subpath->rows;
else
*pNumGroups = estimate_num_groups(subroot,
- get_tlist_exprs(subquery->targetList, false),+ get_tlist_exprs(subroot->parse->targetList, false),
subpath->rows,
NULL,
NULL);
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.cindex 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.hindex 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
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, guofenglinux@gmail.com, 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: <c0a5b39d-8a80-47fb-bd34-6d769f88df8f@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