From 93671bd11f03367c9a0adaefb6d992de3bfde57a Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 31 Mar 2019 00:48:37 +0100 Subject: [PATCH 2/2] fixes and comments --- doc/src/sgml/ref/select.sgml | 6 +++--- src/backend/executor/nodeLimit.c | 24 ++++++++++++++---------- src/backend/nodes/outfuncs.c | 29 +++-------------------------- src/include/nodes/parsenodes.h | 2 +- src/include/optimizer/planmain.h | 2 +- 5 files changed, 22 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index b3b045ea87..e83d309c5b 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -44,7 +44,7 @@ SELECT [ ALL | DISTINCT [ ON ( expressionexpression [ ASC | DESC | USING operator ] [ NULLS { FIRST | LAST } ] [, ...] ] [ LIMIT { count | ALL } ] [ OFFSET start [ ROW | ROWS ] ] - [ FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } [ ONLY | WITH TIES ] ] + [ FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } { ONLY | WITH TIES } ] [ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF table_name [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ] where from_item can be one of: @@ -1430,7 +1430,7 @@ OFFSET start which PostgreSQL also supports. It is: OFFSET start { ROW | ROWS } -FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } [ ONLY | WITH TIES ] +FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } { ONLY | WITH TIES } In this syntax, the start or count value is required by @@ -1442,7 +1442,7 @@ FETCH { FIRST | NEXT } [ count ] { omitted in a FETCH clause, it defaults to 1. ROW . WITH TIES option is used to return two or more rows - that tie for last place in the limit results set according to ORDER BY + that tie for the last place in the result set according to ORDER BY clause (ORDER BY clause must be specified in this case). and ROWS as well as FIRST and NEXT are noise words that don't influence diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c index e8aed10177..68ade28c75 100644 --- a/src/backend/executor/nodeLimit.c +++ b/src/backend/executor/nodeLimit.c @@ -127,9 +127,12 @@ ExecLimit(PlanState *pstate) { /* * Forwards scan, so check for stepping off end of window. If - * we are at the end of the window, return NULL without - * advancing the subplan or the position variable; but change - * the state machine state to record having done so. + * we are at the end of the window, the behavior depends whether + * ONLY or WITH TIES was specified. In case of ONLY, we return + * NULL without advancing the subplan or the position variable; + * but change the state machine state to record having done so. + * In the WITH TIES mode, we need to advance the subplan until + * we find the first row with different ORDER BY pathkeys. */ if (!node->noCount && node->position - node->offset >= node->count && @@ -159,6 +162,7 @@ ExecLimit(PlanState *pstate) node->lstate = LIMIT_SUBPLANEOF; return NULL; } + /* * Test if the new tuple and the last tuple match. * If so we return the tuple. @@ -176,9 +180,9 @@ ExecLimit(PlanState *pstate) node->lstate = LIMIT_WINDOWEND; /* - * If we know we won't need to back up, we can release - * resources at this point. - */ + * If we know we won't need to back up, we can release + * resources at this point. + */ if (!(node->ps.state->es_top_eflags & EXEC_FLAG_BACKWARD)) (void) ExecShutdownNode(outerPlan); @@ -197,12 +201,12 @@ ExecLimit(PlanState *pstate) node->lstate = LIMIT_SUBPLANEOF; return NULL; } - if (node->limitOption == WITH_TIES) - { - ExecCopySlot(node->last_slot, slot); - } node->subSlot = slot; node->position++; + + /* XXX We probably only need to do this for the last tuple in the regular window, no? */ + if (node->limitOption == WITH_TIES) + ExecCopySlot(node->last_slot, slot); } } else diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 2d2ea588d2..6c1d9e4c25 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -910,32 +910,9 @@ _outLimit(StringInfo str, const Limit *node) WRITE_NODE_FIELD(limitCount); WRITE_ENUM_FIELD(limitOption, LimitOption); WRITE_INT_FIELD(numCols); - if (node->numCols > 0) - { - appendStringInfoString(str, " :uniqColIdx"); - for (i = 0; i < node->numCols; i++) - appendStringInfo(str, " %d", node->uniqColIdx[i]); - - appendStringInfoString(str, " :uniqOperators"); - for (i = 0; i < node->numCols; i++) - appendStringInfo(str, " %u", node->uniqOperators[i]); - - appendStringInfoString(str, " :uniqCollations"); - for (i = 0; i < node->numCols; i++) - appendStringInfo(str, " %u", node->uniqOperators[i]); - } - else - { - appendStringInfoString(str, " :uniqColIdx"); - appendStringInfo(str, " NULL"); - - appendStringInfoString(str, " :uniqOperators"); - appendStringInfo(str, " NULL"); - - appendStringInfoString(str, " :uniqCollations"); - appendStringInfo(str, " NULL"); - } - + WRITE_ATTRNUMBER_ARRAY(uniqColIdx, node->numCols); + WRITE_OID_ARRAY(uniqOperators, node->numCols); + WRITE_OID_ARRAY(uniqCollations, node->numCols); } static void diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 4b7357910f..40b9f2eeb8 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -159,7 +159,7 @@ typedef struct Query Node *limitOffset; /* # of result tuples to skip (int8 expr) */ Node *limitCount; /* # of result tuples to return (int8 expr) */ - LimitOption limitOption; /* limit type [WITH TIES | WITH ONLY] */ + LimitOption limitOption; /* limit type {ONLY | WITH TIES} */ List *rowMarks; /* a list of RowMarkClause's */ diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index 52d86bd6fb..924f82ab25 100644 --- a/src/include/optimizer/planmain.h +++ b/src/include/optimizer/planmain.h @@ -57,7 +57,7 @@ extern Agg *make_agg(List *tlist, List *qual, List *groupingSets, List *chain, double dNumGroups, Plan *lefttree); extern Limit *make_limit(Plan *lefttree, Node *limitOffset, Node *limitCount, - LimitOption limitOption,int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations); + LimitOption limitOption, int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations); /* * prototypes for plan/initsplan.c -- 2.20.1