Represent BETWEEN as a special node type in raw parse trees.

Previously, gram.y itself converted BETWEEN into AND (or AND/OR) nests of
expression comparisons.  This was always as bogus as could be, but fixing
it hasn't risen to the top of the to-do list.  The present patch invents an
A_Expr representation for BETWEEN expressions, and does the expansion to
comparison trees in parse_expr.c which is at least a slightly saner place
to be doing semantic conversions.  There should be no change in the post-
parse-analysis results.

This does nothing for the semantic issues with BETWEEN (dubious connection
to btree-opclass semantics, and multiple evaluation of possibly volatile
subexpressions) ... but it's a necessary preliminary step before we could
fix any of that.  The main immediate benefit is that preserving BETWEEN as
an identifiable raw-parse-tree construct will enable better error messages.

While at it, fix the code so that multiply-referenced subexpressions are
physically duplicated before being passed through transformExpr().  This
gets rid of one of the principal reasons why transformExpr() has
historically had to allow already-processed input.

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 84864480624129fb4c3a8e05c9c435ace8960e73..d8e2077d6050f76880ca209d994003b091478479 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2512,6 +2512,22 @@ _outAExpr(StringInfo str, const A_Expr *node)
                        appendStringInfoString(str, " IN ");
                        WRITE_NODE_FIELD(name);
                        break;
+               case AEXPR_BETWEEN:
+                       appendStringInfoString(str, " BETWEEN ");
+                       WRITE_NODE_FIELD(name);
+                       break;
+               case AEXPR_NOT_BETWEEN:
+                       appendStringInfoString(str, " NOT_BETWEEN ");
+                       WRITE_NODE_FIELD(name);
+                       break;
+               case AEXPR_BETWEEN_SYM:
+                       appendStringInfoString(str, " BETWEEN_SYM ");
+                       WRITE_NODE_FIELD(name);
+                       break;
+               case AEXPR_NOT_BETWEEN_SYM:
+                       appendStringInfoString(str, " NOT_BETWEEN_SYM ");
+                       WRITE_NODE_FIELD(name);
+                       break;
                default:
                        appendStringInfoString(str, " ??");
                        break;

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 36dac299144ba8425d42730bae9076814fb164d7..67c7c84deecb4ab05a4f7facb91df32f1ff39c17 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11178,7 +11178,7 @@ a_expr:         c_expr                                                                  { $$ = $1; }
                 * below; and all those operators will have the same precedence.
                 *
                 * If you add more explicitly-known operators, be sure to add them
-                * also to b_expr and to the MathOp list above.
+                * also to b_expr and to the MathOp list below.
                 */
                        | '+' a_expr                                    %prec UMINUS
                                { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", NULL, $2, @1); }
@@ -11396,51 +11396,37 @@ a_expr:               c_expr                                                                  { $$ = $1; }
                                {
                                        $$ = (Node *) makeSimpleA_Expr(AEXPR_OF, "<>", $1, (Node *) $6, @2);
                                }
-                       /*
-                        *      Ideally we would not use hard-wired operators below but
-                        *      instead use opclasses.  However, mixed data types and other
-                        *      issues make this difficult:
-                        *      http://archives.postgresql.org/pgsql-hackers/2008-08/msg01142.php
-                        */
                        | a_expr BETWEEN opt_asymmetric b_expr AND b_expr               %prec BETWEEN
                                {
-                                       $$ = makeAndExpr(
-                                               (Node *) makeSimpleA_Expr(AEXPR_OP, ">=", $1, $4, @2),
-                                               (Node *) makeSimpleA_Expr(AEXPR_OP, "<=", $1, $6, @2),
-                                                                        @2);
+                                       $$ = (Node *) makeSimpleA_Expr(AEXPR_BETWEEN,
+                                                                                                  "BETWEEN",
+                                                                                                  $1,
+                                                                                                  (Node *) list_make2($4, $6),
+                                                                                                  @2);
                                }
                        | a_expr NOT BETWEEN opt_asymmetric b_expr AND b_expr   %prec BETWEEN
                                {
-                                       $$ = makeOrExpr(
-                                               (Node *) makeSimpleA_Expr(AEXPR_OP, "<", $1, $5, @2),
-                                               (Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $7, @2),
-                                                                       @2);
+                                       $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_BETWEEN,
+                                                                                                  "NOT BETWEEN",
+                                                                                                  $1,
+                                                                                                  (Node *) list_make2($5, $7),
+                                                                                                  @2);
                                }
                        | a_expr BETWEEN SYMMETRIC b_expr AND b_expr                    %prec BETWEEN
                                {
-                                       $$ = makeOrExpr(
-                                                 makeAndExpr(
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, ">=", $1, $4, @2),
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, "<=", $1, $6, @2),
-                                                                         @2),
-                                                 makeAndExpr(
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, ">=", $1, $6, @2),
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, "<=", $1, $4, @2),
-                                                                         @2),
-                                                                       @2);
+                                       $$ = (Node *) makeSimpleA_Expr(AEXPR_BETWEEN_SYM,
+                                                                                                  "BETWEEN SYMMETRIC",
+                                                                                                  $1,
+                                                                                                  (Node *) list_make2($4, $6),
+                                                                                                  @2);
                                }
                        | a_expr NOT BETWEEN SYMMETRIC b_expr AND b_expr                %prec BETWEEN
                                {
-                                       $$ = makeAndExpr(
-                                                  makeOrExpr(
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, "<", $1, $5, @2),
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $7, @2),
-                                                                         @2),
-                                                  makeOrExpr(
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, "<", $1, $7, @2),
-                                                       (Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $5, @2),
-                                                                         @2),
-                                                                        @2);
+                                       $$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_BETWEEN_SYM,
+                                                                                                  "NOT BETWEEN SYMMETRIC",
+                                                                                                  $1,
+                                                                                                  (Node *) list_make2($5, $7),
+                                                                                                  @2);
                                }
                        | a_expr IN_P in_expr
                                {

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index f0f0488c57ad52691bb946e7de4c820a08054bb8..67a2310ad0e0e881f94a66cff90f34420700980a 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -48,6 +48,7 @@ static Node *transformAExprDistinct(ParseState *pstate, A_Expr *a);
 static Node *transformAExprNullIf(ParseState *pstate, A_Expr *a);
 static Node *transformAExprOf(ParseState *pstate, A_Expr *a);
 static Node *transformAExprIn(ParseState *pstate, A_Expr *a);
+static Node *transformAExprBetween(ParseState *pstate, A_Expr *a);
 static Node *transformBoolExpr(ParseState *pstate, BoolExpr *a);
 static Node *transformFuncCall(ParseState *pstate, FuncCall *fn);
 static Node *transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref);
@@ -241,6 +242,12 @@ transformExprRecurse(ParseState *pstate, Node *expr)
                                        case AEXPR_IN:
                                                result = transformAExprIn(pstate, a);
                                                break;
+                                       case AEXPR_BETWEEN:
+                                       case AEXPR_NOT_BETWEEN:
+                                       case AEXPR_BETWEEN_SYM:
+                                       case AEXPR_NOT_BETWEEN_SYM:
+                                               result = transformAExprBetween(pstate, a);
+                                               break;
                                        default:
                                                elog(ERROR, "unrecognized A_Expr kind: %d", a->kind);
                                                result = NULL;  /* keep compiler quiet */
@@ -1195,6 +1202,101 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
        return result;
 }
 
+static Node *
+transformAExprBetween(ParseState *pstate, A_Expr *a)
+{
+       Node       *aexpr;
+       Node       *bexpr;
+       Node       *cexpr;
+       Node       *result;
+       Node       *sub1;
+       Node       *sub2;
+       List       *args;
+
+       /* Deconstruct A_Expr into three subexprs */
+       aexpr = a->lexpr;
+       Assert(IsA(a->rexpr, List));
+       args = (List *) a->rexpr;
+       Assert(list_length(args) == 2);
+       bexpr = (Node *) linitial(args);
+       cexpr = (Node *) lsecond(args);
+
+       /*
+        * Build the equivalent comparison expression.  Make copies of
+        * multiply-referenced subexpressions for safety.  (XXX this is really
+        * wrong since it results in multiple runtime evaluations of what may be
+        * volatile expressions ...)
+        *
+        * Ideally we would not use hard-wired operators here but instead use
+        * opclasses.  However, mixed data types and other issues make this
+        * difficult:
+        * http://archives.postgresql.org/pgsql-hackers/2008-08/msg01142.php
+        */
+       switch (a->kind)
+       {
+               case AEXPR_BETWEEN:
+                       args = list_make2(makeSimpleA_Expr(AEXPR_OP, ">=",
+                                                                                          aexpr, bexpr,
+                                                                                          a->location),
+                                                         makeSimpleA_Expr(AEXPR_OP, "<=",
+                                                                                          copyObject(aexpr), cexpr,
+                                                                                          a->location));
+                       result = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
+                       break;
+               case AEXPR_NOT_BETWEEN:
+                       args = list_make2(makeSimpleA_Expr(AEXPR_OP, "<",
+                                                                                          aexpr, bexpr,
+                                                                                          a->location),
+                                                         makeSimpleA_Expr(AEXPR_OP, ">",
+                                                                                          copyObject(aexpr), cexpr,
+                                                                                          a->location));
+                       result = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
+                       break;
+               case AEXPR_BETWEEN_SYM:
+                       args = list_make2(makeSimpleA_Expr(AEXPR_OP, ">=",
+                                                                                          aexpr, bexpr,
+                                                                                          a->location),
+                                                         makeSimpleA_Expr(AEXPR_OP, "<=",
+                                                                                          copyObject(aexpr), cexpr,
+                                                                                          a->location));
+                       sub1 = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
+                       args = list_make2(makeSimpleA_Expr(AEXPR_OP, ">=",
+                                                                               copyObject(aexpr), copyObject(cexpr),
+                                                                                          a->location),
+                                                         makeSimpleA_Expr(AEXPR_OP, "<=",
+                                                                               copyObject(aexpr), copyObject(bexpr),
+                                                                                          a->location));
+                       sub2 = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
+                       args = list_make2(sub1, sub2);
+                       result = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
+                       break;
+               case AEXPR_NOT_BETWEEN_SYM:
+                       args = list_make2(makeSimpleA_Expr(AEXPR_OP, "<",
+                                                                                          aexpr, bexpr,
+                                                                                          a->location),
+                                                         makeSimpleA_Expr(AEXPR_OP, ">",
+                                                                                          copyObject(aexpr), cexpr,
+                                                                                          a->location));
+                       sub1 = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
+                       args = list_make2(makeSimpleA_Expr(AEXPR_OP, "<",
+                                                                               copyObject(aexpr), copyObject(cexpr),
+                                                                                          a->location),
+                                                         makeSimpleA_Expr(AEXPR_OP, ">",
+                                                                               copyObject(aexpr), copyObject(bexpr),
+                                                                                          a->location));
+                       sub2 = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
+                       args = list_make2(sub1, sub2);
+                       result = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
+                       break;
+               default:
+                       elog(ERROR, "unrecognized A_Expr kind: %d", a->kind);
+                       result = NULL;          /* keep compiler quiet */
+                       break;
+       }
+
+       return transformExprRecurse(pstate, result);
+}
+
 static Node *
 transformBoolExpr(ParseState *pstate, BoolExpr *a)
 {

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b1dfa85eb0d703bed9fb633270bdfc21d8739182..35b68ec033252e8a76666fae68efd48fdbd9dda5 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -232,7 +232,11 @@ typedef enum A_Expr_Kind
        AEXPR_DISTINCT,                         /* IS 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 "<>" */
+       AEXPR_IN,                                       /* [NOT] IN - name must be "=" or "<>" */
+       AEXPR_BETWEEN,                          /* name must be "BETWEEN" */
+       AEXPR_NOT_BETWEEN,                      /* name must be "NOT BETWEEN" */
+       AEXPR_BETWEEN_SYM,                      /* name must be "BETWEEN SYMMETRIC" */
+       AEXPR_NOT_BETWEEN_SYM           /* name must be "NOT BETWEEN SYMMETRIC" */
 } A_Expr_Kind;
 
 typedef struct A_Expr