On 2019-Nov-11, Alvaro Herrera wrote:
> I'm not sure the proposed changes to gram.y are all that great, though.
Here's a proposed simplification of the gram.y changes. There are two
things here:
1. cosmetic: we don't need the LimitClause struct; we can use just
SelectLimit, and return that from limit_clause; that can be
complemented using the offset_clause if there's any at select_limit
level.
2. there's a gratuituous palloc() in opt_select_limit when there's no
clause, that seems to be there just so that NULLs can be returned.
That's one extra palloc for SELECTs parsed using one the affected
productions ... it's not every single select, but it seems bad enough
it's worth fixing.
I fixed #2 by just checking whether the return from opt_select_limit is
NULL. ISTM it'd be better to pass the SelectLimit pointer to
insertSelectOptions instead (which is a static function in gram.y anyway
so there's no difficulty there).
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e776215155..f4304a45b9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -135,13 +135,6 @@ typedef struct SelectLimit
LimitOption limitOption;
} SelectLimit;
-/* Private struct for the result of limit_clause production */
-typedef struct LimitClause
-{
- Node *limitCount;
- LimitOption limitOption;
-} LimitClause;
-
/* ConstraintAttributeSpec yields an integer bitmask of these flags: */
#define CAS_NOT_DEFERRABLE 0x01
#define CAS_DEFERRABLE 0x02
@@ -258,8 +251,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
PartitionSpec *partspec;
PartitionBoundSpec *partboundspec;
RoleSpec *rolespec;
- struct SelectLimit *SelectLimit;
- struct LimitClause *LimitClause;
+ struct SelectLimit *selectlimit;
}
%type <node> stmt schema_stmt
@@ -392,8 +384,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <ival> import_qualification_type
%type <importqual> import_qualification
%type <node> vacuum_relation
-%type <SelectLimit> opt_select_limit select_limit
-%type <LimitClause> limit_clause
+%type <selectlimit> opt_select_limit select_limit limit_clause
%type <list> stmtblock stmtmulti
OptTableElementList TableElementList OptInherit definition
@@ -11343,8 +11334,9 @@ select_no_parens:
| select_clause opt_sort_clause for_locking_clause opt_select_limit
{
insertSelectOptions((SelectStmt *) $1, $2, $3,
- ($4)->limitOffset, ($4)->limitCount,
- ($4)->limitOption,
+ ($4) ? ($4)->limitOffset : NULL,
+ ($4) ? ($4)->limitCount : NULL,
+ ($4) ? ($4)->limitOption : LIMIT_OPTION_DEFAULT,
NULL,
yyscanner);
$$ = $1;
@@ -11362,7 +11354,7 @@ select_no_parens:
{
insertSelectOptions((SelectStmt *) $2, NULL, NIL,
NULL, NULL,
- LIMIT_OPTION_DEFAULT,$1,
+ LIMIT_OPTION_DEFAULT, $1,
yyscanner);
$$ = $2;
}
@@ -11370,15 +11362,16 @@ select_no_parens:
{
insertSelectOptions((SelectStmt *) $2, $3, NIL,
NULL, NULL,
- LIMIT_OPTION_DEFAULT,$1,
+ LIMIT_OPTION_DEFAULT, $1,
yyscanner);
$$ = $2;
}
| with_clause select_clause opt_sort_clause for_locking_clause opt_select_limit
{
insertSelectOptions((SelectStmt *) $2, $3, $4,
- ($5)->limitOffset, ($5)->limitCount,
- ($5)->limitOption,
+ ($5) ? ($5)->limitOffset : NULL,
+ ($5) ? ($5)->limitCount : NULL,
+ ($5) ? ($5)->limitOption : LIMIT_OPTION_DEFAULT,
$1,
yyscanner);
$$ = $2;
@@ -11683,27 +11676,17 @@ sortby: a_expr USING qual_all_Op opt_nulls_order
select_limit:
limit_clause offset_clause
{
- SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
- n->limitOffset = $2;
- n->limitCount = ($1)->limitCount;
- n->limitOption = ($1)->limitOption;
- $$ = n;
+ $$ = $1;
+ ($$)->limitOffset = $2;
}
| offset_clause limit_clause
{
- SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
- n->limitOffset = $1;
- n->limitCount = ($2)->limitCount;
- n->limitOption = ($2)->limitOption;
- $$ = n;
+ $$ = $2;
+ ($$)->limitOffset = $1;
}
| limit_clause
{
- SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
- n->limitOffset = NULL;
- n->limitCount = ($1)->limitCount;
- n->limitOption = ($1)->limitOption;
- $$ = n;
+ $$ = $1;
}
| offset_clause
{
@@ -11717,20 +11700,14 @@ select_limit:
opt_select_limit:
select_limit { $$ = $1; }
- | /* EMPTY */
- {
- SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
- n->limitOffset = NULL;
- n->limitCount = NULL;
- n->limitOption = LIMIT_OPTION_DEFAULT;
- $$ = n;
- }
+ | /* EMPTY */ { $$ = NULL; }
;
limit_clause:
LIMIT select_limit_value
{
- LimitClause *n = (LimitClause *) palloc(sizeof(LimitClause));
+ SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+ n->limitOffset = NULL;
n->limitCount = $2;
n->limitOption = LIMIT_OPTION_COUNT;
$$ = n;
@@ -11753,21 +11730,24 @@ limit_clause:
*/
| FETCH first_or_next select_fetch_first_value row_or_rows ONLY
{
- LimitClause *n = (LimitClause *) palloc(sizeof(LimitClause));
+ SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+ n->limitOffset = NULL;
n->limitCount = $3;
n->limitOption = LIMIT_OPTION_COUNT;
$$ = n;
}
| FETCH first_or_next select_fetch_first_value row_or_rows WITH TIES
{
- LimitClause *n = (LimitClause *) palloc(sizeof(LimitClause));
+ SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+ n->limitOffset = NULL;
n->limitCount = $3;
n->limitOption = LIMIT_OPTION_WITH_TIES;
$$ = n;
}
| FETCH first_or_next row_or_rows ONLY
{
- LimitClause *n = (LimitClause *) palloc(sizeof(LimitClause));
+ SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+ n->limitOffset = NULL;
n->limitCount = makeIntConst(1, -1);
n->limitOption = LIMIT_OPTION_COUNT;
$$ = n;
