Suppress unnecessary RelabelType nodes in more cases.

eval_const_expressions sometimes produced RelabelType nodes that
were useless because they just relabeled an expression to the same
exposed type it already had.  This is worth avoiding because it can
cause two equivalent expressions to not be equal(), preventing
recognition of useful optimizations.  In the test case added here,
an unpatched planner fails to notice that the "sqli = constant" clause
renders a sort step unnecessary, because one code path produces an
extra RelabelType and another doesn't.

Fix by ensuring that eval_const_expressions_mutator's T_RelabelType
case will not add in an unnecessary RelabelType.  Also save some
code by sharing a subroutine with the effectively-equivalent cases
for CollateExpr and CoerceToDomain.  (CollateExpr had no bug, and
I think that the case couldn't arise with CoerceToDomain, but
it seems prudent to do the same check for all three cases.)

Back-patch to v12.  In principle this has been wrong all along,
but I haven't seen a case where it causes visible misbehavior
before v12, so refrain from changing stable branches unnecessarily.

Per investigation of a report from Eric Gillum.

Discussion: https://postgr.es/m/CAMmjdmvAZsUEskHYj=KT9sTukVVCiCSoe_PBKOXsncFeAUDPCQ@mail.gmail.com
This commit is contained in:
Tom Lane 2020-02-26 18:13:58 -05:00
parent 3acfe6b089
commit a477bfc1df
3 changed files with 101 additions and 97 deletions

View File

@ -119,6 +119,9 @@ static Node *eval_const_expressions_mutator(Node *node,
static bool contain_non_const_walker(Node *node, void *context);
static bool ece_function_is_safe(Oid funcid,
eval_const_expressions_context *context);
static Node *apply_const_relabel(Node *arg, Oid rtype,
int32 rtypmod, Oid rcollid,
CoercionForm rformat, int rlocation);
static List *simplify_or_arguments(List *args,
eval_const_expressions_context *context,
bool *haveNull, bool *forceTrue);
@ -2774,46 +2777,19 @@ eval_const_expressions_mutator(Node *node,
return node;
case T_RelabelType:
{
/*
* If we can simplify the input to a constant, then we don't
* need the RelabelType node anymore: just change the type
* field of the Const node. Otherwise, must copy the
* RelabelType node.
*/
RelabelType *relabel = (RelabelType *) node;
Node *arg;
/* Simplify the input ... */
arg = eval_const_expressions_mutator((Node *) relabel->arg,
context);
/*
* If we find stacked RelabelTypes (eg, from foo :: int ::
* oid) we can discard all but the top one.
*/
while (arg && IsA(arg, RelabelType))
arg = (Node *) ((RelabelType *) arg)->arg;
if (arg && IsA(arg, Const))
{
Const *con = (Const *) arg;
con->consttype = relabel->resulttype;
con->consttypmod = relabel->resulttypmod;
con->constcollid = relabel->resultcollid;
return (Node *) con;
}
else
{
RelabelType *newrelabel = makeNode(RelabelType);
newrelabel->arg = (Expr *) arg;
newrelabel->resulttype = relabel->resulttype;
newrelabel->resulttypmod = relabel->resulttypmod;
newrelabel->resultcollid = relabel->resultcollid;
newrelabel->relabelformat = relabel->relabelformat;
newrelabel->location = relabel->location;
return (Node *) newrelabel;
}
/* ... and attach a new RelabelType node, if needed */
return apply_const_relabel(arg,
relabel->resulttype,
relabel->resulttypmod,
relabel->resultcollid,
relabel->relabelformat,
relabel->location);
}
case T_CoerceViaIO:
{
@ -2947,48 +2923,25 @@ eval_const_expressions_mutator(Node *node,
case T_CollateExpr:
{
/*
* If we can simplify the input to a constant, then we don't
* need the CollateExpr node at all: just change the
* constcollid field of the Const node. Otherwise, replace
* the CollateExpr with a RelabelType. (We do that so as to
* improve uniformity of expression representation and thus
* simplify comparison of expressions.)
* We replace CollateExpr with RelabelType, so as to improve
* uniformity of expression representation and thus simplify
* comparison of expressions. Hence this looks very nearly
* the same as the RelabelType case, and we can apply the same
* optimizations to avoid unnecessary RelabelTypes.
*/
CollateExpr *collate = (CollateExpr *) node;
Node *arg;
/* Simplify the input ... */
arg = eval_const_expressions_mutator((Node *) collate->arg,
context);
if (arg && IsA(arg, Const))
{
Const *con = (Const *) arg;
con->constcollid = collate->collOid;
return (Node *) con;
}
else if (collate->collOid == exprCollation(arg))
{
/* Don't need a RelabelType either... */
return arg;
}
else
{
RelabelType *relabel = makeNode(RelabelType);
relabel->resulttype = exprType(arg);
relabel->resulttypmod = exprTypmod(arg);
relabel->resultcollid = collate->collOid;
relabel->relabelformat = COERCE_IMPLICIT_CAST;
relabel->location = collate->location;
/* Don't create stacked RelabelTypes */
while (arg && IsA(arg, RelabelType))
arg = (Node *) ((RelabelType *) arg)->arg;
relabel->arg = (Expr *) arg;
return (Node *) relabel;
}
/* ... and attach a new RelabelType node, if needed */
return apply_const_relabel(arg,
exprType(arg),
exprTypmod(arg),
collate->collOid,
COERCE_IMPLICIT_CAST,
collate->location);
}
case T_CaseExpr:
{
@ -3490,32 +3443,12 @@ eval_const_expressions_mutator(Node *node,
cdomain->resulttype);
/* Generate RelabelType to substitute for CoerceToDomain */
/* This should match the RelabelType logic above */
while (arg && IsA(arg, RelabelType))
arg = (Node *) ((RelabelType *) arg)->arg;
if (arg && IsA(arg, Const))
{
Const *con = (Const *) arg;
con->consttype = cdomain->resulttype;
con->consttypmod = cdomain->resulttypmod;
con->constcollid = cdomain->resultcollid;
return (Node *) con;
}
else
{
RelabelType *newrelabel = makeNode(RelabelType);
newrelabel->arg = (Expr *) arg;
newrelabel->resulttype = cdomain->resulttype;
newrelabel->resulttypmod = cdomain->resulttypmod;
newrelabel->resultcollid = cdomain->resultcollid;
newrelabel->relabelformat = cdomain->coercionformat;
newrelabel->location = cdomain->location;
return (Node *) newrelabel;
}
return apply_const_relabel(arg,
cdomain->resulttype,
cdomain->resulttypmod,
cdomain->resultcollid,
cdomain->coercionformat,
cdomain->location);
}
newcdomain = makeNode(CoerceToDomain);
@ -3648,6 +3581,58 @@ ece_function_is_safe(Oid funcid, eval_const_expressions_context *context)
return false;
}
/*
* Subroutine for eval_const_expressions: apply RelabelType if needed
*/
static Node *
apply_const_relabel(Node *arg, Oid rtype, int32 rtypmod, Oid rcollid,
CoercionForm rformat, int rlocation)
{
/*
* If we find stacked RelabelTypes (eg, from foo::int::oid) we can discard
* all but the top one, and must do so to ensure that semantically
* equivalent expressions are equal().
*/
while (arg && IsA(arg, RelabelType))
arg = (Node *) ((RelabelType *) arg)->arg;
if (arg && IsA(arg, Const))
{
/*
* If it's a Const, just modify it in-place; since this is part of
* eval_const_expressions, we want to end up with a simple Const not
* an expression tree. We assume the Const is newly generated and
* hence safe to modify.
*/
Const *con = (Const *) arg;
con->consttype = rtype;
con->consttypmod = rtypmod;
con->constcollid = rcollid;
return (Node *) con;
}
else if (exprType(arg) == rtype &&
exprTypmod(arg) == rtypmod &&
exprCollation(arg) == rcollid)
{
/* Sometimes we find a nest of relabels that net out to nothing. */
return arg;
}
else
{
/* Nope, gotta have a RelabelType. */
RelabelType *newrelabel = makeNode(RelabelType);
newrelabel->arg = (Expr *) arg;
newrelabel->resulttype = rtype;
newrelabel->resulttypmod = rtypmod;
newrelabel->resultcollid = rcollid;
newrelabel->relabelformat = rformat;
newrelabel->location = rlocation;
return (Node *) newrelabel;
}
}
/*
* Subroutine for eval_const_expressions: process arguments of an OR clause
*

View File

@ -439,3 +439,15 @@ explain (costs off)
Filter: ((unique1 = unique1) OR (unique2 = unique2))
(2 rows)
-- check that we recognize equivalence with dummy domains in the way
create temp table undername (f1 name, f2 int);
create temp view overview as
select f1::information_schema.sql_identifier as sqli, f2 from undername;
explain (costs off) -- this should not require a sort
select * from overview where sqli = 'foo' order by sqli;
QUERY PLAN
------------------------------
Seq Scan on undername
Filter: (f1 = 'foo'::name)
(2 rows)

View File

@ -262,3 +262,10 @@ explain (costs off)
-- this could be converted, but isn't at present
explain (costs off)
select * from tenk1 where unique1 = unique1 or unique2 = unique2;
-- check that we recognize equivalence with dummy domains in the way
create temp table undername (f1 name, f2 int);
create temp view overview as
select f1::information_schema.sql_identifier as sqli, f2 from undername;
explain (costs off) -- this should not require a sort
select * from overview where sqli = 'foo' order by sqli;