mirror of
https://github.com/postgres/postgres.git
synced 2025-07-28 23:42:10 +03:00
Don't convert Consts into Vars during setrefs.c processing.
While converting expressions in an upper-level plan node so that they reference Vars and expressions provided by the input plan node(s), don't convert plain Const items, even if there happens to be a matching Const in the input. It's silly to do so because a Var is more expensive to execute than a Const. Moreover, converting can fool ExecCheckPlanOutput's check that an insert or update query inserts nulls into dropped columns, leading to "query provides a value for a dropped column" errors during INSERT or UPDATE on a table with a dropped column. We could solve this by making that check more complicated, but I don't see the point; this fix should save a marginal number of cycles, and it also makes for less messy EXPLAIN output, as shown by the ensuing regression test result changes. Per report from Pavel Hanák. I have not incorporated a test case based on that example, as there doesn't seem to be a simple way of checking this in isolation without making a bunch of assumptions about other planner and SQL-function behavior. Back-patch to 9.6. This setrefs.c behavior exists much further back, but there is not currently reason to think that it causes problems before 9.6. Discussion: <83shraampf.fsf@is-it.eu>
This commit is contained in:
@ -1823,6 +1823,19 @@ set_dummy_tlist_references(Plan *plan, int rtoffset)
|
||||
Var *oldvar = (Var *) tle->expr;
|
||||
Var *newvar;
|
||||
|
||||
/*
|
||||
* As in search_indexed_tlist_for_non_var(), we prefer to keep Consts
|
||||
* as Consts, not Vars referencing Consts. Here, there's no speed
|
||||
* advantage to be had, but it makes EXPLAIN output look cleaner, and
|
||||
* again it avoids confusing the executor.
|
||||
*/
|
||||
if (IsA(oldvar, Const))
|
||||
{
|
||||
/* just reuse the existing TLE node */
|
||||
output_targetlist = lappend(output_targetlist, tle);
|
||||
continue;
|
||||
}
|
||||
|
||||
newvar = makeVar(OUTER_VAR,
|
||||
tle->resno,
|
||||
exprType((Node *) oldvar),
|
||||
@ -2010,6 +2023,16 @@ search_indexed_tlist_for_non_var(Node *node,
|
||||
{
|
||||
TargetEntry *tle;
|
||||
|
||||
/*
|
||||
* If it's a simple Const, replacing it with a Var is silly, even if there
|
||||
* happens to be an identical Const below; a Var is more expensive to
|
||||
* execute than a Const. What's more, replacing it could confuse some
|
||||
* places in the executor that expect to see simple Consts for, eg,
|
||||
* dropped columns.
|
||||
*/
|
||||
if (IsA(node, Const))
|
||||
return NULL;
|
||||
|
||||
tle = tlist_member(node, itlist->tlist);
|
||||
if (tle)
|
||||
{
|
||||
|
Reference in New Issue
Block a user