Skip to content

Commit 34af082

Browse files
tglsfdctglsfdc
committed
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 parent 74811c4 commit 34af082

File tree

4 files changed

+144
-36
lines changed

4 files changed

+144
-36
lines changed

‎src/backend/nodes/outfuncs.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2512,6 +2512,22 @@ _outAExpr(StringInfo str, const A_Expr *node)
25122512
appendStringInfoString(str, " IN ");
25132513
WRITE_NODE_FIELD(name);
25142514
break;
2515+
case AEXPR_BETWEEN:
2516+
appendStringInfoString(str, " BETWEEN ");
2517+
WRITE_NODE_FIELD(name);
2518+
break;
2519+
case AEXPR_NOT_BETWEEN:
2520+
appendStringInfoString(str, " NOT_BETWEEN ");
2521+
WRITE_NODE_FIELD(name);
2522+
break;
2523+
case AEXPR_BETWEEN_SYM:
2524+
appendStringInfoString(str, " BETWEEN_SYM ");
2525+
WRITE_NODE_FIELD(name);
2526+
break;
2527+
case AEXPR_NOT_BETWEEN_SYM:
2528+
appendStringInfoString(str, " NOT_BETWEEN_SYM ");
2529+
WRITE_NODE_FIELD(name);
2530+
break;
25152531
default:
25162532
appendStringInfoString(str, " ??");
25172533
break;

‎src/backend/parser/gram.y

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11178,7 +11178,7 @@ a_expr: c_expr { $$ = $1; }
1117811178
* below; and all those operators will have the same precedence.
1117911179
*
1118011180
* If you add more explicitly-known operators, be sure to add them
11181-
* also to b_expr and to the MathOp list above.
11181+
* also to b_expr and to the MathOp list below.
1118211182
*/
1118311183
| '+' a_expr %prec UMINUS
1118411184
{ $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", NULL, $2, @1); }
@@ -11396,51 +11396,37 @@ a_expr: c_expr { $$ = $1; }
1139611396
{
1139711397
$$ = (Node *) makeSimpleA_Expr(AEXPR_OF, "<>", $1, (Node *) $6, @2);
1139811398
}
11399-
/*
11400-
* Ideally we would not use hard-wired operators below but
11401-
* instead use opclasses. However, mixed data types and other
11402-
* issues make this difficult:
11403-
* http://archives.postgresql.org/pgsql-hackers/2008-08/msg01142.php
11404-
*/
1140511399
| a_expr BETWEEN opt_asymmetric b_expr AND b_expr %prec BETWEEN
1140611400
{
11407-
$$ = makeAndExpr(
11408-
(Node *) makeSimpleA_Expr(AEXPR_OP, ">=", $1, $4, @2),
11409-
(Node *) makeSimpleA_Expr(AEXPR_OP, "<=", $1, $6, @2),
11410-
@2);
11401+
$$ = (Node *) makeSimpleA_Expr(AEXPR_BETWEEN,
11402+
"BETWEEN",
11403+
$1,
11404+
(Node *) list_make2($4, $6),
11405+
@2);
1141111406
}
1141211407
| a_expr NOT BETWEEN opt_asymmetric b_expr AND b_expr %prec BETWEEN
1141311408
{
11414-
$$ = makeOrExpr(
11415-
(Node *) makeSimpleA_Expr(AEXPR_OP, "<", $1, $5, @2),
11416-
(Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $7, @2),
11417-
@2);
11409+
$$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_BETWEEN,
11410+
"NOT BETWEEN",
11411+
$1,
11412+
(Node *) list_make2($5, $7),
11413+
@2);
1141811414
}
1141911415
| a_expr BETWEEN SYMMETRIC b_expr AND b_expr %prec BETWEEN
1142011416
{
11421-
$$ = makeOrExpr(
11422-
makeAndExpr(
11423-
(Node *) makeSimpleA_Expr(AEXPR_OP, ">=", $1, $4, @2),
11424-
(Node *) makeSimpleA_Expr(AEXPR_OP, "<=", $1, $6, @2),
11425-
@2),
11426-
makeAndExpr(
11427-
(Node *) makeSimpleA_Expr(AEXPR_OP, ">=", $1, $6, @2),
11428-
(Node *) makeSimpleA_Expr(AEXPR_OP, "<=", $1, $4, @2),
11429-
@2),
11430-
@2);
11417+
$$ = (Node *) makeSimpleA_Expr(AEXPR_BETWEEN_SYM,
11418+
"BETWEEN SYMMETRIC",
11419+
$1,
11420+
(Node *) list_make2($4, $6),
11421+
@2);
1143111422
}
1143211423
| a_expr NOT BETWEEN SYMMETRIC b_expr AND b_expr %prec BETWEEN
1143311424
{
11434-
$$ = makeAndExpr(
11435-
makeOrExpr(
11436-
(Node *) makeSimpleA_Expr(AEXPR_OP, "<", $1, $5, @2),
11437-
(Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $7, @2),
11438-
@2),
11439-
makeOrExpr(
11440-
(Node *) makeSimpleA_Expr(AEXPR_OP, "<", $1, $7, @2),
11441-
(Node *) makeSimpleA_Expr(AEXPR_OP, ">", $1, $5, @2),
11442-
@2),
11443-
@2);
11425+
$$ = (Node *) makeSimpleA_Expr(AEXPR_NOT_BETWEEN_SYM,
11426+
"NOT BETWEEN SYMMETRIC",
11427+
$1,
11428+
(Node *) list_make2($5, $7),
11429+
@2);
1144411430
}
1144511431
| a_expr IN_P in_expr
1144611432
{

‎src/backend/parser/parse_expr.c

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ static Node *transformAExprDistinct(ParseState *pstate, A_Expr *a);
4848
static Node *transformAExprNullIf(ParseState *pstate, A_Expr *a);
4949
static Node *transformAExprOf(ParseState *pstate, A_Expr *a);
5050
static Node *transformAExprIn(ParseState *pstate, A_Expr *a);
51+
static Node *transformAExprBetween(ParseState *pstate, A_Expr *a);
5152
static Node *transformBoolExpr(ParseState *pstate, BoolExpr *a);
5253
static Node *transformFuncCall(ParseState *pstate, FuncCall *fn);
5354
static Node *transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref);
@@ -241,6 +242,12 @@ transformExprRecurse(ParseState *pstate, Node *expr)
241242
case AEXPR_IN:
242243
result = transformAExprIn(pstate, a);
243244
break;
245+
case AEXPR_BETWEEN:
246+
case AEXPR_NOT_BETWEEN:
247+
case AEXPR_BETWEEN_SYM:
248+
case AEXPR_NOT_BETWEEN_SYM:
249+
result = transformAExprBetween(pstate, a);
250+
break;
244251
default:
245252
elog(ERROR, "unrecognized A_Expr kind: %d", a->kind);
246253
result = NULL; /* keep compiler quiet */
@@ -1195,6 +1202,101 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11951202
return result;
11961203
}
11971204

1205+
static Node *
1206+
transformAExprBetween(ParseState *pstate, A_Expr *a)
1207+
{
1208+
Node *aexpr;
1209+
Node *bexpr;
1210+
Node *cexpr;
1211+
Node *result;
1212+
Node *sub1;
1213+
Node *sub2;
1214+
List *args;
1215+
1216+
/* Deconstruct A_Expr into three subexprs */
1217+
aexpr = a->lexpr;
1218+
Assert(IsA(a->rexpr, List));
1219+
args = (List *) a->rexpr;
1220+
Assert(list_length(args) == 2);
1221+
bexpr = (Node *) linitial(args);
1222+
cexpr = (Node *) lsecond(args);
1223+
1224+
/*
1225+
* Build the equivalent comparison expression. Make copies of
1226+
* multiply-referenced subexpressions for safety. (XXX this is really
1227+
* wrong since it results in multiple runtime evaluations of what may be
1228+
* volatile expressions ...)
1229+
*
1230+
* Ideally we would not use hard-wired operators here but instead use
1231+
* opclasses. However, mixed data types and other issues make this
1232+
* difficult:
1233+
* http://archives.postgresql.org/pgsql-hackers/2008-08/msg01142.php
1234+
*/
1235+
switch (a->kind)
1236+
{
1237+
case AEXPR_BETWEEN:
1238+
args = list_make2(makeSimpleA_Expr(AEXPR_OP, ">=",
1239+
aexpr, bexpr,
1240+
a->location),
1241+
makeSimpleA_Expr(AEXPR_OP, "<=",
1242+
copyObject(aexpr), cexpr,
1243+
a->location));
1244+
result = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
1245+
break;
1246+
case AEXPR_NOT_BETWEEN:
1247+
args = list_make2(makeSimpleA_Expr(AEXPR_OP, "<",
1248+
aexpr, bexpr,
1249+
a->location),
1250+
makeSimpleA_Expr(AEXPR_OP, ">",
1251+
copyObject(aexpr), cexpr,
1252+
a->location));
1253+
result = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
1254+
break;
1255+
case AEXPR_BETWEEN_SYM:
1256+
args = list_make2(makeSimpleA_Expr(AEXPR_OP, ">=",
1257+
aexpr, bexpr,
1258+
a->location),
1259+
makeSimpleA_Expr(AEXPR_OP, "<=",
1260+
copyObject(aexpr), cexpr,
1261+
a->location));
1262+
sub1 = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
1263+
args = list_make2(makeSimpleA_Expr(AEXPR_OP, ">=",
1264+
copyObject(aexpr), copyObject(cexpr),
1265+
a->location),
1266+
makeSimpleA_Expr(AEXPR_OP, "<=",
1267+
copyObject(aexpr), copyObject(bexpr),
1268+
a->location));
1269+
sub2 = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
1270+
args = list_make2(sub1, sub2);
1271+
result = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
1272+
break;
1273+
case AEXPR_NOT_BETWEEN_SYM:
1274+
args = list_make2(makeSimpleA_Expr(AEXPR_OP, "<",
1275+
aexpr, bexpr,
1276+
a->location),
1277+
makeSimpleA_Expr(AEXPR_OP, ">",
1278+
copyObject(aexpr), cexpr,
1279+
a->location));
1280+
sub1 = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
1281+
args = list_make2(makeSimpleA_Expr(AEXPR_OP, "<",
1282+
copyObject(aexpr), copyObject(cexpr),
1283+
a->location),
1284+
makeSimpleA_Expr(AEXPR_OP, ">",
1285+
copyObject(aexpr), copyObject(bexpr),
1286+
a->location));
1287+
sub2 = (Node *) makeBoolExpr(OR_EXPR, args, a->location);
1288+
args = list_make2(sub1, sub2);
1289+
result = (Node *) makeBoolExpr(AND_EXPR, args, a->location);
1290+
break;
1291+
default:
1292+
elog(ERROR, "unrecognized A_Expr kind: %d", a->kind);
1293+
result = NULL; /* keep compiler quiet */
1294+
break;
1295+
}
1296+
1297+
return transformExprRecurse(pstate, result);
1298+
}
1299+
11981300
static Node *
11991301
transformBoolExpr(ParseState *pstate, BoolExpr *a)
12001302
{

‎src/include/nodes/parsenodes.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,11 @@ typedef enum A_Expr_Kind
232232
AEXPR_DISTINCT, /* IS DISTINCT FROM - name must be "=" */
233233
AEXPR_NULLIF, /* NULLIF - name must be "=" */
234234
AEXPR_OF, /* IS [NOT] OF - name must be "=" or "<>" */
235-
AEXPR_IN /* [NOT] IN - name must be "=" or "<>" */
235+
AEXPR_IN, /* [NOT] IN - name must be "=" or "<>" */
236+
AEXPR_BETWEEN, /* name must be "BETWEEN" */
237+
AEXPR_NOT_BETWEEN, /* name must be "NOT BETWEEN" */
238+
AEXPR_BETWEEN_SYM, /* name must be "BETWEEN SYMMETRIC" */
239+
AEXPR_NOT_BETWEEN_SYM /* name must be "NOT BETWEEN SYMMETRIC" */
236240
} A_Expr_Kind;
237241

238242
typedef struct A_Expr

0 commit comments

Comments
 (0)