Обсуждение: SQL:2008 LIMIT/OFFSET

SQL:2008 specifies the following syntax for what we have so far called
LIMIT and OFFSET

SELECT ... [ ORDER BY ... ]
     OFFSET num {ROW|ROWS} FETCH {FIRST|NEXT} [num] {ROW|ROWS} ONLY

For example,

SELECT id, name FROM tab1 ORDER BY id OFFSET 20 ROWS FETCH NEXT 10 ROWS
ONLY;

(I understand this syntax was taken from IBM.)

Supporting this in PostgreSQL poses a couple of parsing challenges that
involve some tradeoffs.  I have attached a draft patch if you want to
follow along.

FETCH must become reserved.  It's in the same position now that LIMIT
and OFFSET are already.  This should be OK because FETCH is already a
well-known SQL command for cursor use.

The trailing {ROW|ROWS} key words plus the fact that the number
specification is optional after FETCH (defaulting to 1) cause some
independent problems because ROWS is unreserved and ROW can introduce an
expression (c_expr even).

If we want to avoid reshuffling the expression syntax (always good to
avoid) and avoid making ROWS reserved, we need to make some arbitrary
restrictions on what kinds of expressions can be used in these clauses.
  Considering that specifying arbitrary expressions in these places
isn't terribly common and the SQL standard only calls for literals, I
hope I have found a good balance that satisfies the letter of the
standard and works well in practice with some parentheses needed in
complicated cases.  But it may be objected to because it creates some
inconsistencies between the traditional and the new syntax in more
complex cases.

Another question, if we want to go this route, is whether we would want
to change the query tree reversing to use the new syntax.

Comments?
diff -ur -x '*.o' ../cvs-pgsql/src/backend/parser/gram.y src/backend/parser/gram.y
--- ../cvs-pgsql/src/backend/parser/gram.y    2008-10-15 14:19:39.000000000 +0300
+++ src/backend/parser/gram.y    2008-10-20 11:39:31.000000000 +0300
@@ -308,6 +308,8 @@
 %type <objtype>    reindex_type drop_type comment_type

 %type <node>    fetch_direction select_limit_value select_offset_value
+                select_offset_value2 opt_select_fetch_first_value
+%type <ival>    row_or_rows first_or_next

 %type <list>    OptSeqOptList SeqOptList
 %type <defelt>    SeqOptElem
@@ -6575,6 +6577,13 @@
                              errhint("Use separate LIMIT and OFFSET clauses."),
                              scanner_errposition(@1)));
                 }
+            /* SQL:2008 syntax variants */
+            | OFFSET select_offset_value2 row_or_rows
+                { $$ = list_make2($2, NULL); }
+            | FETCH first_or_next opt_select_fetch_first_value row_or_rows ONLY
+                { $$ = list_make2(NULL, $3); }
+            | OFFSET select_offset_value2 row_or_rows FETCH first_or_next opt_select_fetch_first_value row_or_rows
ONLY
+                { $$ = list_make2($2, $6); }
         ;

 opt_select_limit:
@@ -6592,10 +6601,28 @@
                 }
         ;

+opt_select_fetch_first_value:
+            SignedIconst        { $$ = makeIntConst($1, @1); }
+            | '(' a_expr ')'    { $$ = $2; }
+            | /*EMPTY*/        { $$ = makeIntConst(1, @$); }
+        ;
+
 select_offset_value:
             a_expr                                    { $$ = $1; }
         ;

+select_offset_value2:
+            c_expr                                    { $$ = $1; }
+        ;
+
+row_or_rows:
+            ROW        { $$ = 0; }
+            | ROWS        { $$ = 0; }
+
+first_or_next:
+            FIRST_P        { $$ = 0; }
+            | NEXT        { $$ = 0; }
+
 group_clause:
             GROUP_P BY expr_list                    { $$ = $3; }
             | /*EMPTY*/                                { $$ = NIL; }
@@ -9214,6 +9241,7 @@
 RoleId:        ColId                                    { $$ = $1; };

 SignedIconst: ICONST                                { $$ = $1; }
+            | '+' ICONST                            { $$ = + $2; }
             | '-' ICONST                            { $$ = - $2; }
         ;

@@ -9346,7 +9374,6 @@
             | EXPLAIN
             | EXTERNAL
             | FAMILY
-            | FETCH
             | FIRST_P
             | FORCE
             | FORWARD
@@ -9636,6 +9663,7 @@
             | END_P
             | EXCEPT
             | FALSE_P
+            | FETCH
             | FOR
             | FOREIGN
             | FROM
diff -ur -x '*.o' ../cvs-pgsql/src/backend/parser/keywords.c src/backend/parser/keywords.c
--- ../cvs-pgsql/src/backend/parser/keywords.c    2008-10-15 14:19:39.000000000 +0300
+++ src/backend/parser/keywords.c    2008-10-19 22:27:28.000000000 +0300
@@ -165,7 +165,7 @@
     {"extract", EXTRACT, COL_NAME_KEYWORD},
     {"false", FALSE_P, RESERVED_KEYWORD},
     {"family", FAMILY, UNRESERVED_KEYWORD},
-    {"fetch", FETCH, UNRESERVED_KEYWORD},
+    {"fetch", FETCH, RESERVED_KEYWORD},
     {"first", FIRST_P, UNRESERVED_KEYWORD},
     {"float", FLOAT_P, COL_NAME_KEYWORD},
     {"for", FOR, RESERVED_KEYWORD},

Peter Eisentraut <peter_e@gmx.net> writes:
> SQL:2008 specifies the following syntax for what we have so far called 
> LIMIT and OFFSET
> SELECT ... [ ORDER BY ... ]
>      OFFSET num {ROW|ROWS} FETCH {FIRST|NEXT} [num] {ROW|ROWS} ONLY

What does the "NEXT" option mean?  I'm a bit worried that this isn't
actually quite equivalent to LIMIT.

> If we want to avoid reshuffling the expression syntax (always good to 
> avoid) and avoid making ROWS reserved, we need to make some arbitrary 
> restrictions on what kinds of expressions can be used in these clauses. 

This syntax seems sufficiently brain-dead that only standards-compliance
fanatics would use it.  Accordingly, limiting it to match the letter
of the standard (literals only) is probably sufficient.

BTW, I think it's a bad idea to assign made-up parse locations, as
you did here:

> +            | /*EMPTY*/        { $$ = makeIntConst(1, @$); }

Just use -1.
        regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> SQL:2008 specifies the following syntax for what we have so far called 
>> LIMIT and OFFSET
>> SELECT ... [ ORDER BY ... ]
>>      OFFSET num {ROW|ROWS} FETCH {FIRST|NEXT} [num] {ROW|ROWS} ONLY
> 
> What does the "NEXT" option mean?

FIRST and NEXT mean exactly the same, namely nothing.

> BTW, I think it's a bad idea to assign made-up parse locations, as
> you did here:
> 
>> +            | /*EMPTY*/        { $$ = makeIntConst(1, @$); }
> 
> Just use -1.

Hmm, @$ is the location of the complete rule, so it should point to the
"empty" spot in theory.  Or am I misunderstanding something?

It's hard to simulate an error with this of course.

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> BTW, I think it's a bad idea to assign made-up parse locations, as
>> you did here:

> Hmm, @$ is the location of the complete rule, so it should point to the
> "empty" spot in theory.  Or am I misunderstanding something?

Well, yeah, but what is that?  If you did get an error complaining
about, say, an invalid integral constant, the cursor wouldn't be
pointing at anything sensible.

I'm not even very sure that bison would produce a valid offset at all in
this case; it looks to me like the location macro just copies a value
that might not have been initialized.  Did you test what gets produced?

But even stipulating that the cursor would point at the next or previous
token, it seems it'd be more confusing than useful.
        regards, tom lane