Skip to content

Commit b89140a

Browse files
tglsfdctglsfdc
committed
Do honest transformation and preprocessing of LIMIT/OFFSET clauses,
instead of the former kluge whereby gram.y emitted already-transformed expressions. This is needed so that Params appearing in these clauses actually work correctly. I suppose some might claim that the side effect of 'SELECT ... LIMIT 2+2' working is a new feature, but I say this is a bug fix.
1 parent 455891b commit b89140a

File tree

8 files changed

+150
-86
lines changed

8 files changed

+150
-86
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.155 2003/06/16 02:03:37 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.156 2003/07/03 19:07:20 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -47,7 +47,8 @@
4747
#define EXPRKIND_QUAL 0
4848
#define EXPRKIND_TARGET 1
4949
#define EXPRKIND_RTFUNC 2
50-
#define EXPRKIND_ININFO 3
50+
#define EXPRKIND_LIMIT 3
51+
#define EXPRKIND_ININFO 4
5152

5253

5354
static Node *preprocess_expression(Query *parse, Node *expr, int kind);
@@ -232,6 +233,11 @@ subquery_planner(Query *parse, double tuple_fraction)
232233
parse->havingQual = preprocess_expression(parse, parse->havingQual,
233234
EXPRKIND_QUAL);
234235

236+
parse->limitOffset = preprocess_expression(parse, parse->limitOffset,
237+
EXPRKIND_LIMIT);
238+
parse->limitCount = preprocess_expression(parse, parse->limitCount,
239+
EXPRKIND_LIMIT);
240+
235241
parse->in_info_list = (List *)
236242
preprocess_expression(parse, (Node *) parse->in_info_list,
237243
EXPRKIND_ININFO);

‎src/backend/optimizer/util/clauses.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.145 2003/07/03 16:33:07 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.146 2003/07/03 19:07:25 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -2390,6 +2390,10 @@ query_tree_walker(Query *query,
23902390
return true;
23912391
if (walker(query->havingQual, context))
23922392
return true;
2393+
if (walker(query->limitOffset, context))
2394+
return true;
2395+
if (walker(query->limitCount, context))
2396+
return true;
23932397
if (walker(query->in_info_list, context))
23942398
return true;
23952399
foreach(rt, query->rtable)
@@ -2863,6 +2867,8 @@ query_tree_mutator(Query *query,
28632867
MUTATE(query->jointree, query->jointree, FromExpr *);
28642868
MUTATE(query->setOperations, query->setOperations, Node *);
28652869
MUTATE(query->havingQual, query->havingQual, Node *);
2870+
MUTATE(query->limitOffset, query->limitOffset, Node *);
2871+
MUTATE(query->limitCount, query->limitCount, Node *);
28662872
MUTATE(query->in_info_list, query->in_info_list, List *);
28672873
FastListInit(&newrt);
28682874
foreach(rt, query->rtable)

‎src/backend/parser/analyze.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.277 2003/06/25 04:19:24 momjian Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.278 2003/07/03 19:07:30 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -459,7 +459,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
459459
qry->distinctClause = NIL;
460460

461461
/* fix where clause */
462-
qual = transformWhereClause(pstate, stmt->whereClause);
462+
qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
463463

464464
/* done building the range table and jointree */
465465
qry->rtable = pstate->p_rtable;
@@ -1588,7 +1588,8 @@ transformIndexStmt(ParseState *pstate, IndexStmt *stmt)
15881588
/* no to join list, yes to namespace */
15891589
addRTEtoQuery(pstate, rte, false, true);
15901590

1591-
stmt->whereClause = transformWhereClause(pstate, stmt->whereClause);
1591+
stmt->whereClause = transformWhereClause(pstate, stmt->whereClause,
1592+
"WHERE");
15921593
}
15931594

15941595
/* take care of any index expressions */
@@ -1699,7 +1700,8 @@ transformRuleStmt(ParseState *pstate, RuleStmt *stmt,
16991700
}
17001701

17011702
/* take care of the where clause */
1702-
stmt->whereClause = transformWhereClause(pstate, stmt->whereClause);
1703+
stmt->whereClause = transformWhereClause(pstate, stmt->whereClause,
1704+
"WHERE");
17031705

17041706
if (length(pstate->p_rtable) != 2) /* naughty, naughty... */
17051707
elog(ERROR, "Rule WHERE condition may not contain references to other relations");
@@ -1891,13 +1893,14 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
18911893
markTargetListOrigins(pstate, qry->targetList);
18921894

18931895
/* transform WHERE */
1894-
qual = transformWhereClause(pstate, stmt->whereClause);
1896+
qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
18951897

18961898
/*
18971899
* Initial processing of HAVING clause is just like WHERE clause.
18981900
* Additional work will be done in optimizer/plan/planner.c.
18991901
*/
1900-
qry->havingQual = transformWhereClause(pstate, stmt->havingClause);
1902+
qry->havingQual = transformWhereClause(pstate, stmt->havingClause,
1903+
"HAVING");
19011904

19021905
/*
19031906
* Transform sorting/grouping stuff. Do ORDER BY first because both
@@ -1918,8 +1921,10 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
19181921
qry->targetList,
19191922
&qry->sortClause);
19201923

1921-
qry->limitOffset = stmt->limitOffset;
1922-
qry->limitCount = stmt->limitCount;
1924+
qry->limitOffset = transformLimitClause(pstate, stmt->limitOffset,
1925+
"OFFSET");
1926+
qry->limitCount = transformLimitClause(pstate, stmt->limitCount,
1927+
"LIMIT");
19231928

19241929
qry->rtable = pstate->p_rtable;
19251930
qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
@@ -2124,8 +2129,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
21242129
if (tllen != length(qry->targetList))
21252130
elog(ERROR, "ORDER BY on a UNION/INTERSECT/EXCEPT result must be on one of the result columns");
21262131

2127-
qry->limitOffset = limitOffset;
2128-
qry->limitCount = limitCount;
2132+
qry->limitOffset = transformLimitClause(pstate, limitOffset,
2133+
"OFFSET");
2134+
qry->limitCount = transformLimitClause(pstate, limitCount,
2135+
"LIMIT");
21292136

21302137
qry->rtable = pstate->p_rtable;
21312138
qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
@@ -2376,7 +2383,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
23762383

23772384
qry->targetList = transformTargetList(pstate, stmt->targetList);
23782385

2379-
qual = transformWhereClause(pstate, stmt->whereClause);
2386+
qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
23802387

23812388
qry->rtable = pstate->p_rtable;
23822389
qry->jointree = makeFromExpr(pstate->p_joinlist, qual);

‎src/backend/parser/gram.y

Lines changed: 10 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.425 2003/07/03 16:33:37 tgl Exp $
14+
* $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.426 2003/07/03 19:07:36 tgl Exp $
1515
*
1616
* HISTORY
1717
* AUTHOR DATE MAJOR EVENT
@@ -4585,80 +4585,32 @@ select_limit:
45854585
| OFFSET select_offset_value
45864586
{ $$ = makeList2($2, NULL); }
45874587
| LIMIT select_limit_value ',' select_offset_value
4588-
/* Disabled because it was too confusing, bjm 2002-02-18 */
4589-
{ elog(ERROR,
4590-
"LIMIT #,# syntax not supported.\n\tUse separate LIMIT and OFFSET clauses."); }
4588+
{
4589+
/* Disabled because it was too confusing, bjm 2002-02-18 */
4590+
elog(ERROR,
4591+
"LIMIT #,# syntax not supported.\n\tUse separate LIMIT and OFFSET clauses.");
4592+
}
45914593
;
45924594

4593-
45944595
opt_select_limit:
45954596
select_limit { $$ = $1; }
45964597
| /* EMPTY */
45974598
{ $$ = makeList2(NULL,NULL); }
45984599
;
45994600

46004601
select_limit_value:
4601-
Iconst
4602-
{
4603-
Const *n = makeNode(Const);
4604-
4605-
if ($1 < 0)
4606-
elog(ERROR, "LIMIT must not be negative");
4607-
4608-
n->consttype = INT4OID;
4609-
n->constlen = sizeof(int4);
4610-
n->constvalue = Int32GetDatum($1);
4611-
n->constisnull = FALSE;
4612-
n->constbyval = TRUE;
4613-
$$ = (Node *)n;
4614-
}
4602+
a_expr { $$ = $1; }
46154603
| ALL
46164604
{
46174605
/* LIMIT ALL is represented as a NULL constant */
4618-
Const *n = makeNode(Const);
4619-
4620-
n->consttype = INT4OID;
4621-
n->constlen = sizeof(int4);
4622-
n->constvalue = (Datum) 0;
4623-
n->constisnull = TRUE;
4624-
n->constbyval = TRUE;
4625-
$$ = (Node *)n;
4626-
}
4627-
| PARAM
4628-
{
4629-
Param *n = makeNode(Param);
4630-
4631-
n->paramkind = PARAM_NUM;
4632-
n->paramid = $1;
4633-
n->paramtype = INT4OID;
4606+
A_Const *n = makeNode(A_Const);
4607+
n->val.type = T_Null;
46344608
$$ = (Node *)n;
46354609
}
46364610
;
46374611

46384612
select_offset_value:
4639-
Iconst
4640-
{
4641-
Const *n = makeNode(Const);
4642-
4643-
if ($1 < 0)
4644-
elog(ERROR, "OFFSET must not be negative");
4645-
4646-
n->consttype = INT4OID;
4647-
n->constlen = sizeof(int4);
4648-
n->constvalue = Int32GetDatum($1);
4649-
n->constisnull = FALSE;
4650-
n->constbyval = TRUE;
4651-
$$ = (Node *)n;
4652-
}
4653-
| PARAM
4654-
{
4655-
Param *n = makeNode(Param);
4656-
4657-
n->paramkind = PARAM_NUM;
4658-
n->paramid = $1;
4659-
n->paramtype = INT4OID;
4660-
$$ = (Node *)n;
4661-
}
4613+
a_expr { $$ = $1; }
46624614
;
46634615

46644616
/*

‎src/backend/parser/parse_clause.c

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.116 2003/06/16 02:03:37 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.117 2003/07/03 19:07:45 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -317,10 +317,7 @@ transformJoinOnClause(ParseState *pstate, JoinExpr *j,
317317
save_namespace = pstate->p_namespace;
318318
pstate->p_namespace = makeList2(j->larg, j->rarg);
319319

320-
/* This part is just like transformWhereClause() */
321-
result = transformExpr(pstate, j->quals);
322-
323-
result = coerce_to_boolean(pstate, result, "JOIN/ON");
320+
result = transformWhereClause(pstate, j->quals, "JOIN/ON");
324321

325322
pstate->p_namespace = save_namespace;
326323

@@ -945,10 +942,38 @@ buildMergedJoinVar(ParseState *pstate, JoinType jointype,
945942

946943
/*
947944
* transformWhereClause -
948-
* transforms the qualification and make sure it is of type Boolean
945+
* Transform the qualification and make sure it is of type boolean.
946+
* Used for WHERE and allied clauses.
947+
*
948+
* constructName does not affect the semantics, but is used in error messages
949+
*/
950+
Node *
951+
transformWhereClause(ParseState *pstate, Node *clause,
952+
const char *constructName)
953+
{
954+
Node *qual;
955+
956+
if (clause == NULL)
957+
return NULL;
958+
959+
qual = transformExpr(pstate, clause);
960+
961+
qual = coerce_to_boolean(pstate, qual, constructName);
962+
963+
return qual;
964+
}
965+
966+
967+
/*
968+
* transformLimitClause -
969+
* Transform the expression and make sure it is of type integer.
970+
* Used for LIMIT and allied clauses.
971+
*
972+
* constructName does not affect the semantics, but is used in error messages
949973
*/
950974
Node *
951-
transformWhereClause(ParseState *pstate, Node *clause)
975+
transformLimitClause(ParseState *pstate, Node *clause,
976+
const char *constructName)
952977
{
953978
Node *qual;
954979

@@ -957,7 +982,31 @@ transformWhereClause(ParseState *pstate, Node *clause)
957982

958983
qual = transformExpr(pstate, clause);
959984

960-
qual = coerce_to_boolean(pstate, qual, "WHERE");
985+
qual = coerce_to_integer(pstate, qual, constructName);
986+
987+
/*
988+
* LIMIT can't refer to any vars or aggregates of the current query;
989+
* we don't allow subselects either (though that case would at least
990+
* be sensible)
991+
*/
992+
if (contain_vars_of_level(qual, 0))
993+
{
994+
/* translator: %s is name of a SQL construct, eg LIMIT */
995+
elog(ERROR, "argument of %s must not contain variables",
996+
constructName);
997+
}
998+
if (checkExprHasAggs(qual))
999+
{
1000+
/* translator: %s is name of a SQL construct, eg LIMIT */
1001+
elog(ERROR, "argument of %s must not contain aggregates",
1002+
constructName);
1003+
}
1004+
if (contain_subplans(qual))
1005+
{
1006+
/* translator: %s is name of a SQL construct, eg LIMIT */
1007+
elog(ERROR, "argument of %s must not contain subselects",
1008+
constructName);
1009+
}
9611010

9621011
return qual;
9631012
}

0 commit comments

Comments
 (0)