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.

1 files changed, 102 insertions, 0 deletions

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index f0f0488c57a..67a2310ad0e 100644
--- 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 */
@@ -1196,6 +1203,101 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
 }
 
 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)
 {
 	List	   *args = NIL;