Re: [HACKERS] psql's help (the LIMIT stuff)

>
> > > >
> > > > I hope the QUERY_LIMIT too.
> > >
> > > I still have that cnfify() possible fix to review for KQSO. Are you
> > > still thinking limit for 6.4 final, or a minor release after that?
> >
> > I posted the part that is the minimum applied to 6.4 to make
> > adding LIMIT later non-initdb earlier. Anyway, here it's
> > again.
>
> Already applied. I assume it is the same as the one I applied.

Seen, thanks. Your 'Applied' just arrived after I packed it
again. It's the same.

> We are close to final, and can easily put it in 6.4.1, which I am sure
> we will need, and if we split CVS trees, you'll have lots of minor
> versions to pick from. :-)
>
> Seems like it would be a nice minor release item, but the problem is
> that minor releases aren't tested as much as major ones. How confident
> are you in the code? What do others thing?

I regression tested it, and did additional tests in the
SPI/PL area. It works. It only touches the parser and the
executor. Rules, planner/optimizer just bypass the values in
the parsetree. The parser and the executor are parts of
Postgres I feel very familiar with (not so in the optimizer).
I trust in the code and would use it in a production
environment.

It's below.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck(at)debis(dot)com (Jan Wieck) #

diff -cr src.orig/backend/commands/command.c src/backend/commands/command.c
*** src.orig/backend/commands/command.c Fri Oct 16 11:53:38 1998
--- src/backend/commands/command.c Fri Oct 16 12:56:44 1998
***************
*** 39,44 ****
--- 39,45 ----
#include "utils/mcxt.h"
#include "utils/portal.h"
#include "utils/syscache.h"
+ #include "string.h"

/* ----------------
* PortalExecutorHeapMemory stuff
***************
*** 101,106 ****
--- 102,108 ----
int feature;
QueryDesc *queryDesc;
MemoryContext context;
+ Const limcount;

/* ----------------
* sanity checks
***************
*** 113,118 ****
--- 115,134 ----
}

/* ----------------
+ * Create a const node from the given count value
+ * ----------------
+ */
+ memset(&limcount, 0, sizeof(limcount));
+ limcount.type = T_Const;
+ limcount.consttype = INT4OID;
+ limcount.constlen = sizeof(int4);
+ limcount.constvalue = (Datum)count;
+ limcount.constisnull = FALSE;
+ limcount.constbyval = TRUE;
+ limcount.constisset = FALSE;
+ limcount.constiscast = FALSE;
+
+ /* ----------------
* get the portal from the portal name
* ----------------
*/
***************
*** 176,182 ****
PortalExecutorHeapMemory = (MemoryContext)
PortalGetHeapMemory(portal);

! ExecutorRun(queryDesc, PortalGetState(portal), feature, count);

if (dest == None) /* MOVE */
pfree(queryDesc);
--- 192,198 ----
PortalExecutorHeapMemory = (MemoryContext)
PortalGetHeapMemory(portal);

! ExecutorRun(queryDesc, PortalGetState(portal), feature, (Node *)NULL, (Node *)&limcount);

if (dest == None) /* MOVE */
pfree(queryDesc);
diff -cr src.orig/backend/executor/execMain.c src/backend/executor/execMain.c
*** src.orig/backend/executor/execMain.c Fri Oct 16 11:53:38 1998
--- src/backend/executor/execMain.c Fri Oct 16 20:05:19 1998
***************
*** 64,69 ****
--- 64,70 ----
static void EndPlan(Plan *plan, EState *estate);
static TupleTableSlot *ExecutePlan(EState *estate, Plan *plan,
Query *parseTree, CmdType operation,
+ int offsetTuples,
int numberTuples, ScanDirection direction,
void (*printfunc) ());
static void ExecRetrieve(TupleTableSlot *slot, void (*printfunc) (),
***************
*** 163,169 ****
* ----------------------------------------------------------------
*/
TupleTableSlot *
! ExecutorRun(QueryDesc *queryDesc, EState *estate, int feature, int count)
{
CmdType operation;
Query *parseTree;
--- 164,170 ----
* ----------------------------------------------------------------
*/
TupleTableSlot *
! ExecutorRun(QueryDesc *queryDesc, EState *estate, int feature, Node *limoffset, Node *limcount)
{
CmdType operation;
Query *parseTree;
***************
*** 171,176 ****
--- 172,179 ----
TupleTableSlot *result;
CommandDest dest;
void (*destination) ();
+ int offset = 0;
+ int count = 0;

/******************
* sanity checks
***************
*** 191,196 ****
--- 194,289 ----
estate->es_processed = 0;
estate->es_lastoid = InvalidOid;

+ /******************
+ * if given get the offset of the LIMIT clause
+ ******************
+ */
+ if (limoffset != NULL)
+ {
+ Const *coffset;
+ Param *poffset;
+ ParamListInfo paramLI;
+ int i;
+
+ switch (nodeTag(limoffset))
+ {
+ case T_Const:
+ coffset = (Const *)limoffset;
+ offset = (int)(coffset->constvalue);
+ break;
+
+ case T_Param:
+ poffset = (Param *)limoffset;
+ paramLI = estate->es_param_list_info;
+
+ if (paramLI == NULL)
+ elog(ERROR, "parameter for limit offset not in executor state");
+ for (i = 0; paramLI[i].kind != PARAM_INVALID; i++)
+ {
+ if (paramLI[i].kind == PARAM_NUM && paramLI[i].id == poffset->paramid)
+ break;
+ }
+ if (paramLI[i].kind == PARAM_INVALID)
+ elog(ERROR, "parameter for limit offset not in executor state");
+ if (paramLI[i].isnull)
+ elog(ERROR, "limit offset cannot be NULL value");
+ offset = (int)(paramLI[i].value);
+
+ break;
+
+ default:
+ elog(ERROR, "unexpected node type %d as limit offset", nodeTag(limoffset));
+ }
+
+ if (offset < 0)
+ elog(ERROR, "limit offset cannot be negative");
+ }
+
+ /******************
+ * if given get the count of the LIMIT clause
+ ******************
+ */
+ if (limcount != NULL)
+ {
+ Const *ccount;
+ Param *pcount;
+ ParamListInfo paramLI;
+ int i;
+
+ switch (nodeTag(limcount))
+ {
+ case T_Const:
+ ccount = (Const *)limcount;
+ count = (int)(ccount->constvalue);
+ break;
+
+ case T_Param:
+ pcount = (Param *)limcount;
+ paramLI = estate->es_param_list_info;
+
+ if (paramLI == NULL)
+ elog(ERROR, "parameter for limit count not in executor state");
+ for (i = 0; paramLI[i].kind != PARAM_INVALID; i++)
+ {
+ if (paramLI[i].kind == PARAM_NUM && paramLI[i].id == pcount->paramid)
+ break;
+ }
+ if (paramLI[i].kind == PARAM_INVALID)
+ elog(ERROR, "parameter for limit count not in executor state");
+ if (paramLI[i].isnull)
+ elog(ERROR, "limit count cannot be NULL value");
+ count = (int)(paramLI[i].value);
+
+ break;
+
+ default:
+ elog(ERROR, "unexpected node type %d as limit count", nodeTag(limcount));
+ }
+
+ if (count < 0)
+ elog(ERROR, "limit count cannot be negative");
+ }
+
switch (feature)
{

***************
*** 199,205 ****
plan,
parseTree,
operation,
! ALL_TUPLES,
ForwardScanDirection,
destination);
break;
--- 292,299 ----
plan,
parseTree,
operation,
! offset,
! count,
ForwardScanDirection,
destination);
break;
***************
*** 208,213 ****
--- 302,308 ----
plan,
parseTree,
operation,
+ offset,
count,
ForwardScanDirection,
destination);
***************
*** 222,227 ****
--- 317,323 ----
plan,
parseTree,
operation,
+ offset,
count,
BackwardScanDirection,
destination);
***************
*** 237,242 ****
--- 333,339 ----
plan,
parseTree,
operation,
+ 0,
ONE_TUPLE,
ForwardScanDirection,
destination);
***************
*** 691,696 ****
--- 788,794 ----
Plan *plan,
Query *parseTree,
CmdType operation,
+ int offsetTuples,
int numberTuples,
ScanDirection direction,
void (*printfunc) ())
***************
*** 742,747 ****
--- 840,859 ----
{
result = NULL;
break;
+ }
+
+ /******************
+ * For now we completely execute the plan and skip
+ * result tuples if requested by LIMIT offset.
+ * Finally we should try to do it in deeper levels
+ * if possible (during index scan)
+ * - Jan
+ ******************
+ */
+ if (offsetTuples > 0)
+ {
+ --offsetTuples;
+ continue;
}

/******************
diff -cr src.orig/backend/executor/functions.c src/backend/executor/functions.c
*** src.orig/backend/executor/functions.c Fri Oct 16 11:53:38 1998
--- src/backend/executor/functions.c Fri Oct 16 19:01:02 1998
***************
*** 130,135 ****
--- 130,138 ----
None);
estate = CreateExecutorState();

+ if (queryTree->limitOffset != NULL || queryTree->limitCount != NULL)
+ elog(ERROR, "LIMIT clause from SQL functions not yet implemented");
+
if (nargs > 0)
{
int i;
***************
*** 200,206 ****

feature = (LAST_POSTQUEL_COMMAND(es)) ? EXEC_RETONE : EXEC_RUN;

! return ExecutorRun(es->qd, es->estate, feature, 0);
}

static void
--- 203,209 ----

feature = (LAST_POSTQUEL_COMMAND(es)) ? EXEC_RETONE : EXEC_RUN;

! return ExecutorRun(es->qd, es->estate, feature, (Node *)NULL, (Node *)NULL);
}

static void
diff -cr src.orig/backend/executor/spi.c src/backend/executor/spi.c
*** src.orig/backend/executor/spi.c Fri Oct 16 11:53:39 1998
--- src/backend/executor/spi.c Fri Oct 16 19:25:33 1998
***************
*** 791,796 ****
--- 791,798 ----
bool isRetrieveIntoRelation = false;
char *intoName = NULL;
int res;
+ Const tcount_const;
+ Node *count = NULL;

switch (operation)
{
***************
*** 825,830 ****
--- 827,865 ----
return SPI_ERROR_OPUNKNOWN;
}

+ /* ----------------
+ * Get the query LIMIT tuple count
+ * ----------------
+ */
+ if (parseTree->limitCount != NULL)
+ {
+ /* ----------------
+ * A limit clause in the parsetree overrides the
+ * tcount parameter
+ * ----------------
+ */
+ count = parseTree->limitCount;
+ }
+ else
+ {
+ /* ----------------
+ * No LIMIT clause in parsetree. Use a local Const node
+ * to put tcount into it
+ * ----------------
+ */
+ memset(&tcount_const, 0, sizeof(tcount_const));
+ tcount_const.type = T_Const;
+ tcount_const.consttype = INT4OID;
+ tcount_const.constlen = sizeof(int4);
+ tcount_const.constvalue = (Datum)tcount;
+ tcount_const.constisnull = FALSE;
+ tcount_const.constbyval = TRUE;
+ tcount_const.constisset = FALSE;
+ tcount_const.constiscast = FALSE;
+
+ count = (Node *)&tcount_const;
+ }
+
if (state == NULL) /* plan preparation */
return res;
#ifdef SPI_EXECUTOR_STATS
***************
*** 845,851 ****
return SPI_OK_CURSOR;
}

! ExecutorRun(queryDesc, state, EXEC_FOR, tcount);

_SPI_current->processed = state->es_processed;
if (operation == CMD_SELECT && queryDesc->dest == SPI)
--- 880,886 ----
return SPI_OK_CURSOR;
}

! ExecutorRun(queryDesc, state, EXEC_FOR, parseTree->limitOffset, count);

_SPI_current->processed = state->es_processed;
if (operation == CMD_SELECT && queryDesc->dest == SPI)
diff -cr src.orig/backend/parser/analyze.c src/backend/parser/analyze.c
*** src.orig/backend/parser/analyze.c Fri Oct 16 11:53:41 1998
--- src/backend/parser/analyze.c Fri Oct 16 13:29:27 1998
***************
*** 180,186 ****
--- 180,190 ----

case T_SelectStmt:
if (!((SelectStmt *) parseTree)->portalname)
+ {
result = transformSelectStmt(pstate, (SelectStmt *) parseTree);
+ result->limitOffset = ((SelectStmt *)parseTree)->limitOffset;
+ result->limitCount = ((SelectStmt *)parseTree)->limitCount;
+ }
else
result = transformCursorStmt(pstate, (SelectStmt *) parseTree);
break;
diff -cr src.orig/backend/parser/gram.y src/backend/parser/gram.y
*** src.orig/backend/parser/gram.y Fri Oct 16 11:53:42 1998
--- src/backend/parser/gram.y Sun Oct 18 22:20:36 1998
***************
*** 45,50 ****
--- 45,51 ----
#include "catalog/catname.h"
#include "utils/elog.h"
#include "access/xact.h"
+ #include "catalog/pg_type.h"

#ifdef MULTIBYTE
#include "mb/pg_wchar.h"
***************
*** 163,169 ****
sort_clause, sortby_list, index_params, index_list, name_list,
from_clause, from_list, opt_array_bounds, nest_array_bounds,
expr_list, attrs, res_target_list, res_target_list2,
! def_list, opt_indirection, group_clause, TriggerFuncArgs

%type <node> func_return
%type <boolean> set_opt
--- 164,171 ----
sort_clause, sortby_list, index_params, index_list, name_list,
from_clause, from_list, opt_array_bounds, nest_array_bounds,
expr_list, attrs, res_target_list, res_target_list2,
! def_list, opt_indirection, group_clause, TriggerFuncArgs,
! opt_select_limit

%type <node> func_return
%type <boolean> set_opt
***************
*** 192,197 ****
--- 194,201 ----

%type <ival> fetch_how_many

+ %type <node> select_limit_value select_offset_value
+
%type <list> OptSeqList
%type <defelt> OptSeqElem

***************
*** 267,273 ****
FALSE_P, FETCH, FLOAT, FOR, FOREIGN, FROM, FULL,
GRANT, GROUP, HAVING, HOUR_P,
IN, INNER_P, INSENSITIVE, INSERT, INTERVAL, INTO, IS,
! JOIN, KEY, LANGUAGE, LEADING, LEFT, LIKE, LOCAL,
MATCH, MINUTE_P, MONTH_P, NAMES,
NATIONAL, NATURAL, NCHAR, NEXT, NO, NOT, NULL_P, NUMERIC,
OF, ON, ONLY, OPTION, OR, ORDER, OUTER_P,
--- 271,277 ----
FALSE_P, FETCH, FLOAT, FOR, FOREIGN, FROM, FULL,
GRANT, GROUP, HAVING, HOUR_P,
IN, INNER_P, INSENSITIVE, INSERT, INTERVAL, INTO, IS,
! JOIN, KEY, LANGUAGE, LEADING, LEFT, LIKE, LIMIT, LOCAL,
MATCH, MINUTE_P, MONTH_P, NAMES,
NATIONAL, NATURAL, NCHAR, NEXT, NO, NOT, NULL_P, NUMERIC,
OF, ON, ONLY, OPTION, OR, ORDER, OUTER_P,
***************
*** 299,305 ****
INCREMENT, INDEX, INHERITS, INSTEAD, ISNULL,
LANCOMPILER, LISTEN, LOAD, LOCATION, LOCK_P, MAXVALUE, MINVALUE, MOVE,
NEW, NOCREATEDB, NOCREATEUSER, NONE, NOTHING, NOTIFY, NOTNULL,
! OIDS, OPERATOR, PASSWORD, PROCEDURAL,
RECIPE, RENAME, RESET, RETURNS, ROW, RULE,
SEQUENCE, SERIAL, SETOF, SHOW, START, STATEMENT, STDIN, STDOUT, TRUSTED,
UNLISTEN, UNTIL, VACUUM, VALID, VERBOSE, VERSION
--- 303,309 ----
INCREMENT, INDEX, INHERITS, INSTEAD, ISNULL,
LANCOMPILER, LISTEN, LOAD, LOCATION, LOCK_P, MAXVALUE, MINVALUE, MOVE,
NEW, NOCREATEDB, NOCREATEUSER, NONE, NOTHING, NOTIFY, NOTNULL,
! OFFSET, OIDS, OPERATOR, PASSWORD, PROCEDURAL,
RECIPE, RENAME, RESET, RETURNS, ROW, RULE,
SEQUENCE, SERIAL, SETOF, SHOW, START, STATEMENT, STDIN, STDOUT, TRUSTED,
UNLISTEN, UNTIL, VACUUM, VALID, VERBOSE, VERSION
***************
*** 2591,2596 ****
--- 2595,2601 ----
result from_clause where_clause
group_clause having_clause
union_clause sort_clause
+ opt_select_limit
{
SelectStmt *n = makeNode(SelectStmt);
n->unique = $2;
***************
*** 2602,2607 ****
--- 2607,2622 ----
n->havingClause = $8;
n->unionClause = $9;
n->sortClause = $10;
+ if ($11 != NIL)
+ {
+ n->limitOffset = nth(0, $11);
+ n->limitCount = nth(1, $11);
+ }
+ else
+ {
+ n->limitOffset = NULL;
+ n->limitCount = NULL;
+ }
$$ = (Node *)n;
}
;
***************
*** 2699,2704 ****
--- 2714,2794 ----
| ASC { $$ = "<"; }
| DESC { $$ = ">"; }
| /*EMPTY*/ { $$ = "<"; /*default*/ }
+ ;
+
+ opt_select_limit: LIMIT select_offset_value ',' select_limit_value
+ { $$ = lappend(lappend(NIL, $2), $4); }
+ | LIMIT select_limit_value OFFSET select_offset_value
+ { $$ = lappend(lappend(NIL, $4), $2); }
+ | LIMIT select_limit_value
+ { $$ = lappend(lappend(NIL, NULL), $2); }
+ | OFFSET select_offset_value LIMIT select_limit_value
+ { $$ = lappend(lappend(NIL, $2), $4); }
+ | OFFSET select_offset_value
+ { $$ = lappend(lappend(NIL, $2), NULL); }
+ | /* EMPTY */
+ { $$ = NIL; }
+ ;
+
+ select_limit_value: Iconst
+ {
+ Const *n = makeNode(Const);
+
+ if ($1 < 1)
+ elog(ERROR, "selection limit must be ALL or a positive integer value > 0");
+
+ n->consttype = INT4OID;
+ n->constlen = sizeof(int4);
+ n->constvalue = (Datum)$1;
+ n->constisnull = FALSE;
+ n->constbyval = TRUE;
+ n->constisset = FALSE;
+ n->constiscast = FALSE;
+ $$ = (Node *)n;
+ }
+ | ALL
+ {
+ Const *n = makeNode(Const);
+ n->consttype = INT4OID;
+ n->constlen = sizeof(int4);
+ n->constvalue = (Datum)0;
+ n->constisnull = FALSE;
+ n->constbyval = TRUE;
+ n->constisset = FALSE;
+ n->constiscast = FALSE;
+ $$ = (Node *)n;
+ }
+ | PARAM
+ {
+ Param *n = makeNode(Param);
+ n->paramkind = PARAM_NUM;
+ n->paramid = $1;
+ n->paramtype = INT4OID;
+ $$ = (Node *)n;
+ }
+ ;
+
+ select_offset_value: Iconst
+ {
+ Const *n = makeNode(Const);
+
+ n->consttype = INT4OID;
+ n->constlen = sizeof(int4);
+ n->constvalue = (Datum)$1;
+ n->constisnull = FALSE;
+ n->constbyval = TRUE;
+ n->constisset = FALSE;
+ n->constiscast = FALSE;
+ $$ = (Node *)n;
+ }
+ | PARAM
+ {
+ Param *n = makeNode(Param);
+ n->paramkind = PARAM_NUM;
+ n->paramid = $1;
+ n->paramtype = INT4OID;
+ $$ = (Node *)n;
+ }
;

/*
diff -cr src.orig/backend/parser/keywords.c src/backend/parser/keywords.c
*** src.orig/backend/parser/keywords.c Fri Oct 16 11:53:42 1998
--- src/backend/parser/keywords.c Sun Oct 18 22:13:29 1998
***************
*** 128,133 ****
--- 128,134 ----
{"leading", LEADING},
{"left", LEFT},
{"like", LIKE},
+ {"limit", LIMIT},
{"listen", LISTEN},
{"load", LOAD},
{"local", LOCAL},
***************
*** 156,161 ****
--- 157,163 ----
{"null", NULL_P},
{"numeric", NUMERIC},
{"of", OF},
+ {"offset", OFFSET},
{"oids", OIDS},
{"old", CURRENT},
{"on", ON},
diff -cr src.orig/backend/rewrite/rewriteDefine.c src/backend/rewrite/rewriteDefine.c
*** src.orig/backend/rewrite/rewriteDefine.c Fri Oct 16 11:53:46 1998
--- src/backend/rewrite/rewriteDefine.c Fri Oct 16 13:48:55 1998
***************
*** 312,317 ****
--- 312,323 ----
heap_close(event_relation);

/*
+ * LIMIT in view is not supported
+ */
+ if (query->limitOffset != NULL || query->limitCount != NULL)
+ elog(ERROR, "LIMIT clause not supported in views");
+
+ /*
* ... and finally the rule must be named _RETviewname.
*/
sprintf(expected_name, "_RET%s", event_obj->relname);
diff -cr src.orig/backend/tcop/pquery.c src/backend/tcop/pquery.c
*** src.orig/backend/tcop/pquery.c Fri Oct 16 11:53:47 1998
--- src/backend/tcop/pquery.c Fri Oct 16 14:02:36 1998
***************
*** 40,46 ****
#include "commands/command.h"

static char *CreateOperationTag(int operationType);
! static void ProcessQueryDesc(QueryDesc *queryDesc);


/* ----------------------------------------------------------------
--- 40,46 ----
#include "commands/command.h"

static char *CreateOperationTag(int operationType);
! static void ProcessQueryDesc(QueryDesc *queryDesc, Node *limoffset, Node *limcount);


/* ----------------------------------------------------------------
***************
*** 205,211 ****
* ----------------------------------------------------------------
*/
static void
! ProcessQueryDesc(QueryDesc *queryDesc)
{
Query *parseTree;
Plan *plan;
--- 205,211 ----
* ----------------------------------------------------------------
*/
static void
! ProcessQueryDesc(QueryDesc *queryDesc, Node *limoffset, Node *limcount)
{
Query *parseTree;
Plan *plan;
***************
*** 330,336 ****
* actually run the plan..
* ----------------
*/
! ExecutorRun(queryDesc, state, EXEC_RUN, 0);

/* save infos for EndCommand */
UpdateCommandInfo(operation, state->es_lastoid, state->es_processed);
--- 330,336 ----
* actually run the plan..
* ----------------
*/
! ExecutorRun(queryDesc, state, EXEC_RUN, limoffset, limcount);

/* save infos for EndCommand */
UpdateCommandInfo(operation, state->es_lastoid, state->es_processed);
***************
*** 373,377 ****
print_plan(plan, parsetree);
}
else
! ProcessQueryDesc(queryDesc);
}
--- 373,377 ----
print_plan(plan, parsetree);
}
else
! ProcessQueryDesc(queryDesc, parsetree->limitOffset, parsetree->limitCount);
}
diff -cr src.orig/include/executor/executor.h src/include/executor/executor.h
*** src.orig/include/executor/executor.h Fri Oct 16 11:53:56 1998
--- src/include/executor/executor.h Fri Oct 16 12:04:17 1998
***************
*** 83,89 ****
* prototypes from functions in execMain.c
*/
extern TupleDesc ExecutorStart(QueryDesc *queryDesc, EState *estate);
! extern TupleTableSlot *ExecutorRun(QueryDesc *queryDesc, EState *estate, int feature, int count);
extern void ExecutorEnd(QueryDesc *queryDesc, EState *estate);
extern HeapTuple ExecConstraints(char *caller, Relation rel, HeapTuple tuple);
#ifdef QUERY_LIMIT
--- 83,89 ----
* prototypes from functions in execMain.c
*/
extern TupleDesc ExecutorStart(QueryDesc *queryDesc, EState *estate);
! extern TupleTableSlot *ExecutorRun(QueryDesc *queryDesc, EState *estate, int feature, Node *limoffset, Node *limcount);
extern void ExecutorEnd(QueryDesc *queryDesc, EState *estate);
extern HeapTuple ExecConstraints(char *caller, Relation rel, HeapTuple tuple);
#ifdef QUERY_LIMIT