From 8d19d0e139238cdcb3f1f7e1adc4ff959562822f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 28 Jul 2016 17:23:03 -0400 Subject: [PATCH] Teach parser to transform "x IS [NOT] DISTINCT FROM NULL" to a NullTest. Now that we've nailed down the principle that NullTest with !argisrow is fully equivalent to SQL's IS [NOT] DISTINCT FROM NULL, let's teach the parser about it. This produces a slightly more compact parse tree and is much more amenable to optimization than a DistinctExpr, since the planner knows a good deal about NullTest and next to nothing about DistinctExpr. I'm not sure that there are all that many queries in the wild that could be improved by this, but at least one source of such cases is the patch just made to postgres_fdw to emit IS [NOT] DISTINCT FROM NULL when IS [NOT] NULL isn't semantically correct. No back-patch, since to the extent that this does affect planning results, it might be considered undesirable plan destabilization. --- src/backend/nodes/outfuncs.c | 4 ++ src/backend/parser/gram.y | 8 +--- src/backend/parser/parse_expr.c | 68 ++++++++++++++++++++++++++++----- src/include/nodes/parsenodes.h | 1 + 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index e196622737..acaf4ea5eb 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2903,6 +2903,10 @@ _outAExpr(StringInfo str, const A_Expr *node) appendStringInfoString(str, " DISTINCT "); WRITE_NODE_FIELD(name); break; + case AEXPR_NOT_DISTINCT: + appendStringInfoString(str, " NOT_DISTINCT "); + WRITE_NODE_FIELD(name); + break; case AEXPR_NULLIF: appendStringInfoString(str, " NULLIF "); WRITE_NODE_FIELD(name); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index edf4516dac..0cae44641f 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11839,9 +11839,7 @@ a_expr: c_expr { $$ = $1; } } | a_expr IS NOT DISTINCT FROM a_expr %prec IS { - $$ = makeNotExpr((Node *) makeSimpleA_Expr(AEXPR_DISTINCT, - "=", $1, $6, @2), - @2); + $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_DISTINCT, "=", $1, $6, @2); } | a_expr IS OF '(' type_list ')' %prec IS { @@ -12025,9 +12023,7 @@ b_expr: c_expr } | b_expr IS NOT DISTINCT FROM b_expr %prec IS { - $$ = makeNotExpr((Node *) makeSimpleA_Expr(AEXPR_DISTINCT, - "=", $1, $6, @2), - @2); + $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_DISTINCT, "=", $1, $6, @2); } | b_expr IS OF '(' type_list ')' %prec IS { diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 8b285165d5..cead21283d 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -124,6 +124,8 @@ static Node *make_row_distinct_op(ParseState *pstate, List *opname, RowExpr *lrow, RowExpr *rrow, int location); static Expr *make_distinct_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree, int location); +static Node *make_nulltest_from_distinct(ParseState *pstate, + A_Expr *distincta, Node *arg); static int operator_precedence_group(Node *node, const char **nodename); static void emit_precedence_warnings(ParseState *pstate, int opgroup, const char *opname, @@ -224,6 +226,7 @@ transformExprRecurse(ParseState *pstate, Node *expr) result = transformAExprOpAll(pstate, a); break; case AEXPR_DISTINCT: + case AEXPR_NOT_DISTINCT: result = transformAExprDistinct(pstate, a); break; case AEXPR_NULLIF: @@ -991,12 +994,23 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a) { Node *lexpr = a->lexpr; Node *rexpr = a->rexpr; + Node *result; if (operator_precedence_warning) emit_precedence_warnings(pstate, PREC_GROUP_INFIX_IS, "IS", lexpr, rexpr, a->location); + /* + * If either input is an undecorated NULL literal, transform to a NullTest + * on the other input. That's simpler to process than a full DistinctExpr, + * and it avoids needing to require that the datatype have an = operator. + */ + if (exprIsNullConstant(rexpr)) + return make_nulltest_from_distinct(pstate, a, lexpr); + if (exprIsNullConstant(lexpr)) + return make_nulltest_from_distinct(pstate, a, rexpr); + lexpr = transformExprRecurse(pstate, lexpr); rexpr = transformExprRecurse(pstate, rexpr); @@ -1004,20 +1018,31 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a) rexpr && IsA(rexpr, RowExpr)) { /* ROW() op ROW() is handled specially */ - return make_row_distinct_op(pstate, a->name, - (RowExpr *) lexpr, - (RowExpr *) rexpr, - a->location); + result = make_row_distinct_op(pstate, a->name, + (RowExpr *) lexpr, + (RowExpr *) rexpr, + a->location); } else { /* Ordinary scalar operator */ - return (Node *) make_distinct_op(pstate, - a->name, - lexpr, - rexpr, - a->location); + result = (Node *) make_distinct_op(pstate, + a->name, + lexpr, + rexpr, + a->location); } + + /* + * If it's NOT DISTINCT, we first build a DistinctExpr and then stick a + * NOT on top. + */ + if (a->kind == AEXPR_NOT_DISTINCT) + result = (Node *) makeBoolExpr(NOT_EXPR, + list_make1(result), + a->location); + + return result; } static Node * @@ -2869,6 +2894,28 @@ make_distinct_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree, return result; } +/* + * Produce a NullTest node from an IS [NOT] DISTINCT FROM NULL construct + * + * "arg" is the untransformed other argument + */ +static Node * +make_nulltest_from_distinct(ParseState *pstate, A_Expr *distincta, Node *arg) +{ + NullTest *nt = makeNode(NullTest); + + nt->arg = (Expr *) transformExprRecurse(pstate, arg); + /* the argument can be any type, so don't coerce it */ + if (distincta->kind == AEXPR_NOT_DISTINCT) + nt->nulltesttype = IS_NULL; + else + nt->nulltesttype = IS_NOT_NULL; + /* argisrow = false is correct whether or not arg is composite */ + nt->argisrow = false; + nt->location = distincta->location; + return (Node *) nt; +} + /* * Identify node's group for operator precedence warnings * @@ -2971,7 +3018,8 @@ operator_precedence_group(Node *node, const char **nodename) *nodename = strVal(llast(aexpr->name)); group = PREC_GROUP_POSTFIX_OP; } - else if (aexpr->kind == AEXPR_DISTINCT) + else if (aexpr->kind == AEXPR_DISTINCT || + aexpr->kind == AEXPR_NOT_DISTINCT) { *nodename = "IS"; group = PREC_GROUP_INFIX_IS; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 3773dd9c2f..1481fff57d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -237,6 +237,7 @@ typedef enum A_Expr_Kind AEXPR_OP_ANY, /* scalar op ANY (array) */ AEXPR_OP_ALL, /* scalar op ALL (array) */ AEXPR_DISTINCT, /* IS DISTINCT FROM - name must be "=" */ + AEXPR_NOT_DISTINCT, /* IS NOT DISTINCT FROM - name must be "=" */ AEXPR_NULLIF, /* NULLIF - name must be "=" */ AEXPR_OF, /* IS [NOT] OF - name must be "=" or "<>" */ AEXPR_IN, /* [NOT] IN - name must be "=" or "<>" */ -- 2.40.0