Skip to content

Commit 71e58dc

Browse files
tglsfdctglsfdc
committed
Make equal() ignore CoercionForm fields for better planning with casts.
This change ensures that the planner will see implicit and explicit casts as equivalent for all purposes, except in the minority of cases where there's actually a semantic difference (as reflected by having a 3-argument cast function). In particular, this fixes cases where the EquivalenceClass machinery failed to consider two references to a varchar column as equivalent if one was implicitly cast to text but the other was explicitly cast to text, as seen in bug #7598 from Vaclav Juza. We have had similar bugs before in other parts of the planner, so I think it's time to fix this problem at the core instead of continuing to band-aid around it. Remove set_coercionform_dontcare(), which represents the band-aid previously in use for allowing matching of index and constraint expressions with inconsistent cast labeling. (We can probably get rid of COERCE_DONTCARE altogether, but I don't think removing that enum value in back branches would be wise; it's possible there's third party code referring to it.) Back-patch to 9.2. We could go back further, and might want to once this has been tested more; but for the moment I won't risk destabilizing plan choices in long-since-stable branches.
1 parent e583ffe commit 71e58dc

File tree

6 files changed

+17
-132
lines changed

6 files changed

+17
-132
lines changed

‎src/backend/nodes/equalfuncs.c

Lines changed: 11 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@
8383
#define COMPARE_LOCATION_FIELD(fldname) \
8484
((void) 0)
8585

86+
/* Compare a CoercionForm field (also a no-op, per comment in primnodes.h) */
87+
#define COMPARE_COERCIONFORM_FIELD(fldname) \
88+
((void) 0)
89+
8690

8791
/*
8892
* Stuff from primnodes.h
@@ -235,16 +239,7 @@ _equalFuncExpr(const FuncExpr *a, const FuncExpr *b)
235239
COMPARE_SCALAR_FIELD(funcid);
236240
COMPARE_SCALAR_FIELD(funcresulttype);
237241
COMPARE_SCALAR_FIELD(funcretset);
238-
239-
/*
240-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
241-
* that are equal() to both explicit and implicit coercions.
242-
*/
243-
if (a->funcformat != b->funcformat &&
244-
a->funcformat != COERCE_DONTCARE &&
245-
b->funcformat != COERCE_DONTCARE)
246-
return false;
247-
242+
COMPARE_COERCIONFORM_FIELD(funcformat);
248243
COMPARE_SCALAR_FIELD(funccollid);
249244
COMPARE_SCALAR_FIELD(inputcollid);
250245
COMPARE_NODE_FIELD(args);
@@ -448,16 +443,7 @@ _equalRelabelType(const RelabelType *a, const RelabelType *b)
448443
COMPARE_SCALAR_FIELD(resulttype);
449444
COMPARE_SCALAR_FIELD(resulttypmod);
450445
COMPARE_SCALAR_FIELD(resultcollid);
451-
452-
/*
453-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
454-
* that are equal() to both explicit and implicit coercions.
455-
*/
456-
if (a->relabelformat != b->relabelformat &&
457-
a->relabelformat != COERCE_DONTCARE &&
458-
b->relabelformat != COERCE_DONTCARE)
459-
return false;
460-
446+
COMPARE_COERCIONFORM_FIELD(relabelformat);
461447
COMPARE_LOCATION_FIELD(location);
462448

463449
return true;
@@ -469,16 +455,7 @@ _equalCoerceViaIO(const CoerceViaIO *a, const CoerceViaIO *b)
469455
COMPARE_NODE_FIELD(arg);
470456
COMPARE_SCALAR_FIELD(resulttype);
471457
COMPARE_SCALAR_FIELD(resultcollid);
472-
473-
/*
474-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
475-
* that are equal() to both explicit and implicit coercions.
476-
*/
477-
if (a->coerceformat != b->coerceformat &&
478-
a->coerceformat != COERCE_DONTCARE &&
479-
b->coerceformat != COERCE_DONTCARE)
480-
return false;
481-
458+
COMPARE_COERCIONFORM_FIELD(coerceformat);
482459
COMPARE_LOCATION_FIELD(location);
483460

484461
return true;
@@ -493,16 +470,7 @@ _equalArrayCoerceExpr(const ArrayCoerceExpr *a, const ArrayCoerceExpr *b)
493470
COMPARE_SCALAR_FIELD(resulttypmod);
494471
COMPARE_SCALAR_FIELD(resultcollid);
495472
COMPARE_SCALAR_FIELD(isExplicit);
496-
497-
/*
498-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
499-
* that are equal() to both explicit and implicit coercions.
500-
*/
501-
if (a->coerceformat != b->coerceformat &&
502-
a->coerceformat != COERCE_DONTCARE &&
503-
b->coerceformat != COERCE_DONTCARE)
504-
return false;
505-
473+
COMPARE_COERCIONFORM_FIELD(coerceformat);
506474
COMPARE_LOCATION_FIELD(location);
507475

508476
return true;
@@ -513,16 +481,7 @@ _equalConvertRowtypeExpr(const ConvertRowtypeExpr *a, const ConvertRowtypeExpr *
513481
{
514482
COMPARE_NODE_FIELD(arg);
515483
COMPARE_SCALAR_FIELD(resulttype);
516-
517-
/*
518-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
519-
* that are equal() to both explicit and implicit coercions.
520-
*/
521-
if (a->convertformat != b->convertformat &&
522-
a->convertformat != COERCE_DONTCARE &&
523-
b->convertformat != COERCE_DONTCARE)
524-
return false;
525-
484+
COMPARE_COERCIONFORM_FIELD(convertformat);
526485
COMPARE_LOCATION_FIELD(location);
527486

528487
return true;
@@ -589,16 +548,7 @@ _equalRowExpr(const RowExpr *a, const RowExpr *b)
589548
{
590549
COMPARE_NODE_FIELD(args);
591550
COMPARE_SCALAR_FIELD(row_typeid);
592-
593-
/*
594-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
595-
* that are equal() to both explicit and implicit coercions.
596-
*/
597-
if (a->row_format != b->row_format &&
598-
a->row_format != COERCE_DONTCARE &&
599-
b->row_format != COERCE_DONTCARE)
600-
return false;
601-
551+
COMPARE_COERCIONFORM_FIELD(row_format);
602552
COMPARE_NODE_FIELD(colnames);
603553
COMPARE_LOCATION_FIELD(location);
604554

@@ -684,16 +634,7 @@ _equalCoerceToDomain(const CoerceToDomain *a, const CoerceToDomain *b)
684634
COMPARE_SCALAR_FIELD(resulttype);
685635
COMPARE_SCALAR_FIELD(resulttypmod);
686636
COMPARE_SCALAR_FIELD(resultcollid);
687-
688-
/*
689-
* Special-case COERCE_DONTCARE, so that planner can build coercion nodes
690-
* that are equal() to both explicit and implicit coercions.
691-
*/
692-
if (a->coercionformat != b->coercionformat &&
693-
a->coercionformat != COERCE_DONTCARE &&
694-
b->coercionformat != COERCE_DONTCARE)
695-
return false;
696-
637+
COMPARE_COERCIONFORM_FIELD(coercionformat);
697638
COMPARE_LOCATION_FIELD(location);
698639

699640
return true;

‎src/backend/optimizer/util/clauses.c

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ static bool contain_leaky_functions_walker(Node *node, void *context);
9898
static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
9999
static List *find_nonnullable_vars_walker(Node *node, bool top_level);
100100
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
101-
static bool set_coercionform_dontcare_walker(Node *node, void *context);
102101
static Node *eval_const_expressions_mutator(Node *node,
103102
eval_const_expressions_context *context);
104103
static List *simplify_or_arguments(List *args,
@@ -2114,47 +2113,6 @@ strip_implicit_coercions(Node *node)
21142113
return node;
21152114
}
21162115

2117-
/*
2118-
* set_coercionform_dontcare: set all CoercionForm fields to COERCE_DONTCARE
2119-
*
2120-
* This is used to make index expressions and index predicates more easily
2121-
* comparable to clauses of queries. CoercionForm is not semantically
2122-
* significant (for cases where it does matter, the significant info is
2123-
* coded into the coercion function arguments) so we can ignore it during
2124-
* comparisons. Thus, for example, an index on "foo::int4" can match an
2125-
* implicit coercion to int4.
2126-
*
2127-
* Caution: the passed expression tree is modified in-place.
2128-
*/
2129-
void
2130-
set_coercionform_dontcare(Node *node)
2131-
{
2132-
(void) set_coercionform_dontcare_walker(node, NULL);
2133-
}
2134-
2135-
static bool
2136-
set_coercionform_dontcare_walker(Node *node, void *context)
2137-
{
2138-
if (node == NULL)
2139-
return false;
2140-
if (IsA(node, FuncExpr))
2141-
((FuncExpr *) node)->funcformat = COERCE_DONTCARE;
2142-
else if (IsA(node, RelabelType))
2143-
((RelabelType *) node)->relabelformat = COERCE_DONTCARE;
2144-
else if (IsA(node, CoerceViaIO))
2145-
((CoerceViaIO *) node)->coerceformat = COERCE_DONTCARE;
2146-
else if (IsA(node, ArrayCoerceExpr))
2147-
((ArrayCoerceExpr *) node)->coerceformat = COERCE_DONTCARE;
2148-
else if (IsA(node, ConvertRowtypeExpr))
2149-
((ConvertRowtypeExpr *) node)->convertformat = COERCE_DONTCARE;
2150-
else if (IsA(node, RowExpr))
2151-
((RowExpr *) node)->row_format = COERCE_DONTCARE;
2152-
else if (IsA(node, CoerceToDomain))
2153-
((CoerceToDomain *) node)->coercionformat = COERCE_DONTCARE;
2154-
return expression_tree_walker(node, set_coercionform_dontcare_walker,
2155-
context);
2156-
}
2157-
21582116
/*
21592117
* Helper for eval_const_expressions: check that datatype of an attribute
21602118
* is still what it was when the expression was parsed. This is needed to

‎src/backend/optimizer/util/plancat.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -665,12 +665,6 @@ get_relation_constraints(PlannerInfo *root,
665665

666666
cexpr = (Node *) canonicalize_qual((Expr *) cexpr);
667667

668-
/*
669-
* Also mark any coercion format fields as "don't care", so that
670-
* we can match to both explicit and implicit coercions.
671-
*/
672-
set_coercionform_dontcare(cexpr);
673-
674668
/* Fix Vars to have the desired varno */
675669
if (varno != 1)
676670
ChangeVarNodes(cexpr, 1, varno, 0);

‎src/backend/utils/cache/relcache.c

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3549,12 +3549,6 @@ RelationGetIndexExpressions(Relation relation)
35493549
*/
35503550
result = (List *) eval_const_expressions(NULL, (Node *) result);
35513551

3552-
/*
3553-
* Also mark any coercion format fields as "don't care", so that the
3554-
* planner can match to both explicit and implicit coercions.
3555-
*/
3556-
set_coercionform_dontcare((Node *) result);
3557-
35583552
/* May as well fix opfuncids too */
35593553
fix_opfuncids((Node *) result);
35603554

@@ -3621,12 +3615,6 @@ RelationGetIndexPredicate(Relation relation)
36213615

36223616
result = (List *) canonicalize_qual((Expr *) result);
36233617

3624-
/*
3625-
* Also mark any coercion format fields as "don't care", so that the
3626-
* planner can match to both explicit and implicit coercions.
3627-
*/
3628-
set_coercionform_dontcare((Node *) result);
3629-
36303618
/* Also convert to implicit-AND format */
36313619
result = make_ands_implicit((Expr *) result);
36323620

‎src/include/nodes/primnodes.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,12 @@ typedef enum CoercionContext
317317

318318
/*
319319
* CoercionForm - information showing how to display a function-call node
320+
*
321+
* NB: equal() ignores CoercionForm fields, therefore this *must* not carry
322+
* any semantically significant information. We need that behavior so that
323+
* the planner will consider equivalent implicit and explicit casts to be
324+
* equivalent. In cases where those actually behave differently, the coercion
325+
* function's arguments will be different.
320326
*/
321327
typedef enum CoercionForm
322328
{

‎src/include/optimizer/clauses.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,6 @@ extern void CommuteRowCompareExpr(RowCompareExpr *clause);
7979

8080
extern Node *strip_implicit_coercions(Node *node);
8181

82-
extern void set_coercionform_dontcare(Node *node);
83-
8482
extern Node *eval_const_expressions(PlannerInfo *root, Node *node);
8583

8684
extern Node *estimate_expression_value(PlannerInfo *root, Node *node);

0 commit comments

Comments
 (0)