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.

Author: tglsfdc

src/backend/optimizer/plan/planner.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.155 2003/06/16 02:03:37 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.156 2003/07/03 19:07:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,7 +47,8 @@
 #define EXPRKIND_QUAL	0
 #define EXPRKIND_TARGET	1
 #define EXPRKIND_RTFUNC	2
-#define EXPRKIND_ININFO	3
+#define EXPRKIND_LIMIT	3
+#define EXPRKIND_ININFO	4
 
 
 static Node *preprocess_expression(Query *parse, Node *expr, int kind);
@@ -232,6 +233,11 @@ subquery_planner(Query *parse, double tuple_fraction)
 	parse->havingQual = preprocess_expression(parse, parse->havingQual,
 											  EXPRKIND_QUAL);
 
+	parse->limitOffset = preprocess_expression(parse, parse->limitOffset,
+											   EXPRKIND_LIMIT);
+	parse->limitCount = preprocess_expression(parse, parse->limitCount,
+											  EXPRKIND_LIMIT);
+
 	parse->in_info_list = (List *)
 		preprocess_expression(parse, (Node *) parse->in_info_list,
 							  EXPRKIND_ININFO);

src/backend/optimizer/util/clauses.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.145 2003/07/03 16:33:07 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.146 2003/07/03 19:07:25 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -2390,6 +2390,10 @@ query_tree_walker(Query *query,
 		return true;
 	if (walker(query->havingQual, context))
 		return true;
+	if (walker(query->limitOffset, context))
+		return true;
+	if (walker(query->limitCount, context))
+		return true;
 	if (walker(query->in_info_list, context))
 		return true;
 	foreach(rt, query->rtable)
@@ -2863,6 +2867,8 @@ query_tree_mutator(Query *query,
 	MUTATE(query->jointree, query->jointree, FromExpr *);
 	MUTATE(query->setOperations, query->setOperations, Node *);
 	MUTATE(query->havingQual, query->havingQual, Node *);
+	MUTATE(query->limitOffset, query->limitOffset, Node *);
+	MUTATE(query->limitCount, query->limitCount, Node *);
 	MUTATE(query->in_info_list, query->in_info_list, List *);
 	FastListInit(&newrt);
 	foreach(rt, query->rtable)

src/backend/parser/analyze.c

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.277 2003/06/25 04:19:24 momjian Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.278 2003/07/03 19:07:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -459,7 +459,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
 	qry->distinctClause = NIL;
 
 	/* fix where clause */
-	qual = transformWhereClause(pstate, stmt->whereClause);
+	qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
 
 	/* done building the range table and jointree */
 	qry->rtable = pstate->p_rtable;
@@ -1588,7 +1588,8 @@ transformIndexStmt(ParseState *pstate, IndexStmt *stmt)
 		/* no to join list, yes to namespace */
 		addRTEtoQuery(pstate, rte, false, true);
 
-		stmt->whereClause = transformWhereClause(pstate, stmt->whereClause);
+		stmt->whereClause = transformWhereClause(pstate, stmt->whereClause,
+												 "WHERE");
 	}
 
 	/* take care of any index expressions */
@@ -1699,7 +1700,8 @@ transformRuleStmt(ParseState *pstate, RuleStmt *stmt,
 	}
 
 	/* take care of the where clause */
-	stmt->whereClause = transformWhereClause(pstate, stmt->whereClause);
+	stmt->whereClause = transformWhereClause(pstate, stmt->whereClause,
+											 "WHERE");
 
 	if (length(pstate->p_rtable) != 2)	/* naughty, naughty... */
 		elog(ERROR, "Rule WHERE condition may not contain references to other relations");
@@ -1891,13 +1893,14 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 	markTargetListOrigins(pstate, qry->targetList);
 
 	/* transform WHERE */
-	qual = transformWhereClause(pstate, stmt->whereClause);
+	qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
 
 	/*
 	 * Initial processing of HAVING clause is just like WHERE clause.
 	 * Additional work will be done in optimizer/plan/planner.c.
 	 */
-	qry->havingQual = transformWhereClause(pstate, stmt->havingClause);
+	qry->havingQual = transformWhereClause(pstate, stmt->havingClause,
+										   "HAVING");
 
 	/*
 	 * Transform sorting/grouping stuff.  Do ORDER BY first because both
@@ -1918,8 +1921,10 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 												  qry->targetList,
 												  &qry->sortClause);
 
-	qry->limitOffset = stmt->limitOffset;
-	qry->limitCount = stmt->limitCount;
+	qry->limitOffset = transformLimitClause(pstate, stmt->limitOffset,
+											"OFFSET");
+	qry->limitCount = transformLimitClause(pstate, stmt->limitCount,
+										   "LIMIT");
 
 	qry->rtable = pstate->p_rtable;
 	qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
@@ -2124,8 +2129,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 	if (tllen != length(qry->targetList))
 		elog(ERROR, "ORDER BY on a UNION/INTERSECT/EXCEPT result must be on one of the result columns");
 
-	qry->limitOffset = limitOffset;
-	qry->limitCount = limitCount;
+	qry->limitOffset = transformLimitClause(pstate, limitOffset,
+											"OFFSET");
+	qry->limitCount = transformLimitClause(pstate, limitCount,
+										   "LIMIT");
 
 	qry->rtable = pstate->p_rtable;
 	qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
@@ -2376,7 +2383,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
 
 	qry->targetList = transformTargetList(pstate, stmt->targetList);
 
-	qual = transformWhereClause(pstate, stmt->whereClause);
+	qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
 
 	qry->rtable = pstate->p_rtable;
 	qry->jointree = makeFromExpr(pstate->p_joinlist, qual);

src/backend/parser/gram.y

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.425 2003/07/03 16:33:37 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.426 2003/07/03 19:07:36 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -4585,80 +4585,32 @@ select_limit:
 			| OFFSET select_offset_value
 				{ $$ = makeList2($2, NULL); }
 			| LIMIT select_limit_value ',' select_offset_value
-				/* Disabled because it was too confusing, bjm 2002-02-18 */
-				{ elog(ERROR,
-					"LIMIT #,# syntax not supported.\n\tUse separate LIMIT and OFFSET clauses."); }
+				{
+					/* Disabled because it was too confusing, bjm 2002-02-18 */
+					elog(ERROR,
+						 "LIMIT #,# syntax not supported.\n\tUse separate LIMIT and OFFSET clauses.");
+				}
 		;
 
-
 opt_select_limit:
 			select_limit							{ $$ = $1; }
 			| /* EMPTY */
 					{ $$ = makeList2(NULL,NULL); }
 		;
 
 select_limit_value:
-			Iconst
-				{
-					Const	*n = makeNode(Const);
-
-					if ($1 < 0)
-						elog(ERROR, "LIMIT must not be negative");
-
-					n->consttype	= INT4OID;
-					n->constlen		= sizeof(int4);
-					n->constvalue	= Int32GetDatum($1);
-					n->constisnull	= FALSE;
-					n->constbyval	= TRUE;
-					$$ = (Node *)n;
-				}
+			a_expr									{ $$ = $1; }
 			| ALL
 				{
 					/* LIMIT ALL is represented as a NULL constant */
-					Const	*n = makeNode(Const);
-
-					n->consttype	= INT4OID;
-					n->constlen		= sizeof(int4);
-					n->constvalue	= (Datum) 0;
-					n->constisnull	= TRUE;
-					n->constbyval	= TRUE;
-					$$ = (Node *)n;
-				}
-			| PARAM
-				{
-					Param	*n = makeNode(Param);
-
-					n->paramkind	= PARAM_NUM;
-					n->paramid		= $1;
-					n->paramtype	= INT4OID;
+					A_Const *n = makeNode(A_Const);
+					n->val.type = T_Null;
 					$$ = (Node *)n;
 				}
 		;
 
 select_offset_value:
-			Iconst
-				{
-					Const	*n = makeNode(Const);
-
-					if ($1 < 0)
-						elog(ERROR, "OFFSET must not be negative");
-
-					n->consttype	= INT4OID;
-					n->constlen		= sizeof(int4);
-					n->constvalue	= Int32GetDatum($1);
-					n->constisnull	= FALSE;
-					n->constbyval	= TRUE;
-					$$ = (Node *)n;
-				}
-			| PARAM
-				{
-					Param	*n = makeNode(Param);
-
-					n->paramkind	= PARAM_NUM;
-					n->paramid		= $1;
-					n->paramtype	= INT4OID;
-					$$ = (Node *)n;
-				}
+			a_expr									{ $$ = $1; }
 		;
 
 /*

src/backend/parser/parse_clause.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.116 2003/06/16 02:03:37 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.117 2003/07/03 19:07:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -317,10 +317,7 @@ transformJoinOnClause(ParseState *pstate, JoinExpr *j,
 	save_namespace = pstate->p_namespace;
 	pstate->p_namespace = makeList2(j->larg, j->rarg);
 
-	/* This part is just like transformWhereClause() */
-	result = transformExpr(pstate, j->quals);
-
-	result = coerce_to_boolean(pstate, result, "JOIN/ON");
+	result = transformWhereClause(pstate, j->quals, "JOIN/ON");
 
 	pstate->p_namespace = save_namespace;
 
@@ -945,10 +942,38 @@ buildMergedJoinVar(ParseState *pstate, JoinType jointype,
 
 /*
  * transformWhereClause -
- *	  transforms the qualification and make sure it is of type Boolean
+ *	  Transform the qualification and make sure it is of type boolean.
+ *	  Used for WHERE and allied clauses.
+ *
+ * constructName does not affect the semantics, but is used in error messages
+ */
+Node *
+transformWhereClause(ParseState *pstate, Node *clause,
+					 const char *constructName)
+{
+	Node	   *qual;
+
+	if (clause == NULL)
+		return NULL;
+
+	qual = transformExpr(pstate, clause);
+
+	qual = coerce_to_boolean(pstate, qual, constructName);
+
+	return qual;
+}
+
+
+/*
+ * transformLimitClause -
+ *	  Transform the expression and make sure it is of type integer.
+ *	  Used for LIMIT and allied clauses.
+ *
+ * constructName does not affect the semantics, but is used in error messages
  */
 Node *
-transformWhereClause(ParseState *pstate, Node *clause)
+transformLimitClause(ParseState *pstate, Node *clause,
+					 const char *constructName)
 {
 	Node	   *qual;
 
@@ -957,7 +982,31 @@ transformWhereClause(ParseState *pstate, Node *clause)
 
 	qual = transformExpr(pstate, clause);
 
-	qual = coerce_to_boolean(pstate, qual, "WHERE");
+	qual = coerce_to_integer(pstate, qual, constructName);
+
+	/*
+	 * LIMIT can't refer to any vars or aggregates of the current query;
+	 * we don't allow subselects either (though that case would at least
+	 * be sensible)
+	 */
+	if (contain_vars_of_level(qual, 0))
+	{
+		/* translator: %s is name of a SQL construct, eg LIMIT */
+		elog(ERROR, "argument of %s must not contain variables",
+			 constructName);
+	}
+	if (checkExprHasAggs(qual))
+	{
+		/* translator: %s is name of a SQL construct, eg LIMIT */
+		elog(ERROR, "argument of %s must not contain aggregates",
+			 constructName);
+	}
+	if (contain_subplans(qual))
+	{
+		/* translator: %s is name of a SQL construct, eg LIMIT */
+		elog(ERROR, "argument of %s must not contain subselects",
+			 constructName);
+	}
 
 	return qual;
 }