8000 Make equal() ignore CoercionForm fields for better planning with casts. · postgres/postgres@71e58dc · GitHub
[go: up one dir, main page]
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
  • plancat.c
  • utils/cache
  • include

    • 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)
    0