From: Andrei Lepikhov <lepihov@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Richard Guo <guofenglinux@gmail.com>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18652: Planner can not find pathkey item to sort for query with expression and expression index
Date: Sat, 12 Oct 2024 13:05:25 +0700
Message-ID: <73d3a8bb-531d-4db3-be75-e9ba7001a3f3@gmail.com> (raw)
In-Reply-To: <3001952.1728680364@sss.pgh.pa.us>
References: <18652-deaa782ebcca85d1@postgresql.org>
<2527690.1728510183@sss.pgh.pa.us>
<CAMbWs49Pcnm_cX=7azYY9d8+qRrOOKDjJDCa0KGRk9kWWMOz3A@mail.gmail.com>
<40c8a5c2-0040-4fe6-ab11-6dca23353a07@gmail.com>
<3001952.1728680364@sss.pgh.pa.us>
On 12/10/2024 03:59, Tom Lane wrote:
> Andrei Lepikhov <lepihov@gmail.com> writes:
> But that's not what actually happens in the sole use of this code by
> prepare_sort_from_pathkeys. What will actually happen is that we
> will add the child EC member expression as a new member of the
> same tlist, so that the plan node's tlist will look like
>
> t2.i + 1, (t2.i + 1) + 0
I ended up with the same chain of events. During this scrutiny, I found
only a few sources of knowledge about why the code was written exactly
this way and what logic is under the hood. So, the current explanation
and discussion are profitable by themselves.
> I wonder whether we're doing more work here than we really need to.
> If the underlying table t1 has columns i and j, and we have an EC
> member that references t1.j while the tlist only mentions t1.i,
> wouldn't it still work to add t1.j to the tlist? So maybe groveling
> through the tlist members is unnecessary and we only need to be
> performing some kind of relation-level check on whether all the
> required relations are included in the input.
I'm not sure I understand this point. Do you mean something like
attached? I thought about that, but what if we have all the relations
under, but some columns (especially ones with no source relation) needed
are absent in the target list and used far below in the plan, or not
used at all?
The patch looks good.
--
regards, Andrei Lepikhov
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.cindex 8f6f005ecb..1c0deab0f6 100644--- a/src/backend/optimizer/path/equivclass.c+++ b/src/backend/optimizer/path/equivclass.c@@ -833,12 +841,13 @@ find_computable_ec_member(PlannerInfo *root,
bool require_parallel_safe)
{
ListCell *lc;
+ Relids r;++ r = pull_varnos(root, (Node *) exprs);
foreach(lc, ec->ec_members)
{
EquivalenceMember *em = (EquivalenceMember *) lfirst(lc);
- List *exprvars;- ListCell *lc2;
/*
* We shouldn't be trying to sort by an equivalence class that
@@ -854,21 +863,8 @@ find_computable_ec_member(PlannerInfo *root,
!bms_is_subset(em->em_relids, relids))
continue;
- /*- * Match if all Vars and quasi-Vars are available in "exprs".- */- exprvars = pull_var_clause((Node *) em->em_expr,- PVC_INCLUDE_AGGREGATES |- PVC_INCLUDE_WINDOWFUNCS |- PVC_INCLUDE_PLACEHOLDERS);- foreach(lc2, exprvars)- {- if (!is_exprlist_member(lfirst(lc2), exprs))- break;- }- list_free(exprvars);- if (lc2)- continue; /* we hit a non-available Var */+ if (!bms_is_subset(em->em_relids, r))+ continue;
/*
* If requested, reject expressions that are not parallel-safe. We
@@ -884,31 +880,6 @@ find_computable_ec_member(PlannerInfo *root,
return NULL;
}
-/*- * is_exprlist_member- * Subroutine for find_computable_ec_member: is "node" in "exprs"?- *- * Per the requirements of that function, "exprs" might or might not have- * TargetEntry superstructure.- */-static bool-is_exprlist_member(Expr *node, List *exprs)-{- ListCell *lc;-- foreach(lc, exprs)- {- Expr *expr = (Expr *) lfirst(lc);-- if (expr && IsA(expr, TargetEntry))- expr = ((TargetEntry *) expr)->expr;-- if (equal(node, expr))- return true;- }- return false;-}-
/*
* relation_can_be_sorted_early
* Can this relation be sorted on this EC before the final output step?
Attachments:
[text/plain] variant.diff (2.0K, ../73d3a8bb-531d-4db3-be75-e9ba7001a3f3@gmail.com/2-variant.diff)
download | inline diff:diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.cindex 8f6f005ecb..1c0deab0f6 100644--- a/src/backend/optimizer/path/equivclass.c+++ b/src/backend/optimizer/path/equivclass.c@@ -833,12 +841,13 @@ find_computable_ec_member(PlannerInfo *root,
bool require_parallel_safe)
{
ListCell *lc;
+ Relids r;++ r = pull_varnos(root, (Node *) exprs);
foreach(lc, ec->ec_members)
{
EquivalenceMember *em = (EquivalenceMember *) lfirst(lc);
- List *exprvars;- ListCell *lc2;
/*
* We shouldn't be trying to sort by an equivalence class that
@@ -854,21 +863,8 @@ find_computable_ec_member(PlannerInfo *root,
!bms_is_subset(em->em_relids, relids))
continue;
- /*- * Match if all Vars and quasi-Vars are available in "exprs".- */- exprvars = pull_var_clause((Node *) em->em_expr,- PVC_INCLUDE_AGGREGATES |- PVC_INCLUDE_WINDOWFUNCS |- PVC_INCLUDE_PLACEHOLDERS);- foreach(lc2, exprvars)- {- if (!is_exprlist_member(lfirst(lc2), exprs))- break;- }- list_free(exprvars);- if (lc2)- continue; /* we hit a non-available Var */+ if (!bms_is_subset(em->em_relids, r))+ continue;
/*
* If requested, reject expressions that are not parallel-safe. We
@@ -884,31 +880,6 @@ find_computable_ec_member(PlannerInfo *root,
return NULL;
}
-/*- * is_exprlist_member- * Subroutine for find_computable_ec_member: is "node" in "exprs"?- *- * Per the requirements of that function, "exprs" might or might not have- * TargetEntry superstructure.- */-static bool-is_exprlist_member(Expr *node, List *exprs)-{- ListCell *lc;-- foreach(lc, exprs)- {- Expr *expr = (Expr *) lfirst(lc);-- if (expr && IsA(expr, TargetEntry))- expr = ((TargetEntry *) expr)->expr;-- if (equal(node, expr))- return true;- }- return false;-}-
/*
* relation_can_be_sorted_early
* Can this relation be sorted on this EC before the final output step?
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: lepihov@gmail.com, tgl@sss.pgh.pa.us, guofenglinux@gmail.com, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18652: Planner can not find pathkey item to sort for query with expression and expression index
In-Reply-To: <73d3a8bb-531d-4db3-be75-e9ba7001a3f3@gmail.com>
* 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