Обсуждение: Updatable views/with check option parsing
I have spent some time figuring out how to resolve the parsing conflicts in Bernd Helmle's updatable views patch. The problem has now been reduced to specifically this situation: CREATE VIEW foo AS SELECT expr :: TIME . WITH (where expr is a_expr or b_expr and TIME could also be TIMESTAMP or TIME(x) or TIMESTAMP(x)). The continuation here could be WITH TIME ZONE (calling for a shift) or WITH CHECK OPTION (calling for a reduce). All the usual ideas about unfolding the rules or making keywords more reserved don't work (why should they). A one-token lookahead simply can't parse this. I have had some ideas about trying to play around with the precedence rules -- giving WITH TIME ZONE a higher precedence than WITH CHECK OPTION -- but I have no experience with that and I am apparently not doing it right, if that is supposed to work at all. If we can't get that to work, it seems that we are out of options unless we want to just accept the conflicts. How should we go about this, and what should Bernd do with his patch, which, as I understand it, has been held up for quite a while simply because he is concerned about this issue? -- Peter Eisentraut http://developer.postgresql.org/~petere/
Ühel kenal päeval, K, 2006-05-24 kell 13:13, kirjutas Peter Eisentraut: > I have spent some time figuring out how to resolve the parsing conflicts in > Bernd Helmle's updatable views patch. The problem has now been reduced to > specifically this situation: > > CREATE VIEW foo AS SELECT expr :: TIME . WITH > > (where expr is a_expr or b_expr and TIME could also be TIMESTAMP or TIME(x) or > TIMESTAMP(x)). > > The continuation here could be WITH TIME ZONE (calling for a shift) or WITH > CHECK OPTION (calling for a reduce). > > All the usual ideas about unfolding the rules or making keywords more reserved > don't work (why should they). A one-token lookahead simply can't parse this. Can't we teach tokenized a new token "WITH TIME ZONE" ? > I have had some ideas about trying to play around with the precedence rules -- > giving WITH TIME ZONE a higher precedence than WITH CHECK OPTION -- but I > have no experience with that and I am apparently not doing it right, if that > is supposed to work at all. > > If we can't get that to work, it seems that we are out of options unless we > want to just accept the conflicts. > > How should we go about this, and what should Bernd do with his patch, which, > as I understand it, has been held up for quite a while simply because he is > concerned about this issue? > -- ---------------- Hannu Krosing Database Architect Skype Technologies OÜ Akadeemia tee 21 F, Tallinn, 12618, Estonia Skype me: callto:hkrosing Get Skype for free: http://www.skype.com
Hannu Krosing wrote: > Can't we teach tokenized a new token "WITH TIME ZONE" ? > No, that's three tokens, not one. We surely don't want to start making white space significant. cheers andrew
Peter Eisentraut <peter_e@gmx.net> writes:
> I have spent some time figuring out how to resolve the parsing conflicts in
> Bernd Helmle's updatable views patch. The problem has now been reduced to
> specifically this situation:
Could we see the proposed patches for gram.y?
> If we can't get that to work, it seems that we are out of options unless we
> want to just accept the conflicts.
Not acceptable, per prior discussions any time someone was too lazy to
fix their grammar patch completely ...
regards, tom lane
Am Mittwoch, 24. Mai 2006 20:42 schrieb Tom Lane: > Peter Eisentraut <peter_e@gmx.net> writes: > > I have spent some time figuring out how to resolve the parsing conflicts > > in Bernd Helmle's updatable views patch. The problem has now been > > reduced to specifically this situation: > > Could we see the proposed patches for gram.y? Here it is. $ make -W gram.y gram.c bison -y -d gram.y conflicts: 4 shift/reduce These are basically for instances of the same problem. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: > Am Mittwoch, 24. Mai 2006 20:42 schrieb Tom Lane: > >> Peter Eisentraut <peter_e@gmx.net> writes: >> >>> I have spent some time figuring out how to resolve the parsing conflicts >>> in Bernd Helmle's updatable views patch. The problem has now been >>> reduced to specifically this situation: >>> >> Could we see the proposed patches for gram.y? >> > > Here it is. > > $ make -W gram.y gram.c > bison -y -d gram.y > conflicts: 4 shift/reduce > > These are basically for instances of the same problem. > I had a quick look - I don't think there is an easy answer with the current proposed grammar. If we want to prevent shift/reduce conflicts I suspect we'd need to use a different keyword than WITH, although I can't think of one that couldn't be a trailing clause on a select statment, which is the cause of the trouble. Another possibility would be to move the optional WITH clause so that it would come before the AS clause. Then there should be no conflict, I believe. Something like: ViewStmt: CREATE OptTemp VIEW qualified_name opt_column_list opt_check_option AS SelectStmt cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> I had a quick look - I don't think there is an easy answer with the
> current proposed grammar. If we want to prevent shift/reduce conflicts I
> suspect we'd need to use a different keyword than WITH, although I can't
> think of one that couldn't be a trailing clause on a select statment,
> which is the cause of the trouble. Another possibility would be to move
> the optional WITH clause so that it would come before the AS clause.
Unfortunately the SQL99 spec is perfectly clear about what it wants:
<view definition> ::= CREATE [ RECURSIVE ] VIEW <table name> <view specification>
AS <query expression> [ WITH [ <levels clause> ] CHECK OPTION ]
<levels clause> ::= CASCADED | LOCAL
I haven't had time to play with this yet, but I suspect the answer will
have to be that we reinstate the token-merging UNION JOIN kluge that I
just took out :-(. Or we could look into recognizing the whole thing as
one token in scan.l, but I suspect that doesn't work unless we give up
the no-backtrack property of the lexer, which would be more of a speed
hit than the intermediate function was. Anyway it should certainly be
soluble with token merging, if we can't find a pure grammar solution.
regards, tom lane
On Wed, May 24, 2006 at 01:13:06PM +0200, Peter Eisentraut wrote: > CREATE VIEW foo AS SELECT expr :: TIME . WITH > > (where expr is a_expr or b_expr and TIME could also be TIMESTAMP or TIME(x) or > TIMESTAMP(x)). > > The continuation here could be WITH TIME ZONE (calling for a shift) or WITH > CHECK OPTION (calling for a reduce). > > All the usual ideas about unfolding the rules or making keywords more reserved > don't work (why should they). A one-token lookahead simply can't parse this. > > I have had some ideas about trying to play around with the precedence rules -- > giving WITH TIME ZONE a higher precedence than WITH CHECK OPTION -- but I > have no experience with that and I am apparently not doing it right, if that > is supposed to work at all. All precedence rules do is force the parser to either shift or reduce without complaining about a conflict. i.e. it resolves the conflict by forcing a particular option. So all you would acheive with precedence rules is that you codify the solution. For example: always shift and if the user specifies WITH CHECK they get a parse error. It would be nice to be able to detect this and tell the user to parenthesise the (expr::TIME) thus solving the problem. Given that :: is not SQL-standard anyway, perhaps this is not a bad solution. Incidently, IIRC the default behaviour on conflict is a shift anyway, so that what the patch already does anyway. So we get: CREATE VIEW foo AS SELECT expr :: TIME WITH TIME ZONE <-- OK CREATE VIEW foo AS SELECT expr :: TIME WITH CHECK OPTION <-- parse error CREATE VIEW foo AS SELECT (expr :: TIME) WITH CHECK OPTION <-- OK Of course, any code that decompiles into SQL will have to be careful to not produce unparseable SQL. Have a ncie day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Martijn van Oosterhout <kleptog@svana.org> writes: > So we get: > > CREATE VIEW foo AS SELECT expr :: TIME WITH TIME ZONE <-- OK > CREATE VIEW foo AS SELECT expr :: TIME WITH CHECK OPTION <-- parse error > CREATE VIEW foo AS SELECT (expr :: TIME) WITH CHECK OPTION <-- OK I haven't really been following this conversation, but just on the off chance this is a useful idea: Would it work to make "WITH" just a noise word? then you would just need one token of look-ahead to recognize "TIME ZONE" or "CHECK OPTION" instead of 2. I don't know what <levels clause>s look like so I'm not sure if you would be able to recognize them without seeing the WITH. I'm not even sure this works even if you can for that matter. -- greg
Martijn van Oosterhout wrote: > Incidently, IIRC the default behaviour on conflict is a shift anyway, > so that what the patch already does anyway. > > So we get: > > CREATE VIEW foo AS SELECT expr :: TIME WITH TIME ZONE <-- OK > CREATE VIEW foo AS SELECT expr :: TIME WITH CHECK OPTION <-- > parse error > CREATE VIEW foo AS SELECT (expr :: TIME) WITH CHECK OPTION <-- OK Yes, that's really the fundamental problem if you let shift/reduce conflicts stand: the parser will behave weirdly in the conflict cases. There is a seemingly little known option in bison named %glr-parser, which when turned on parses all of theses cases correctly. Maybe that is worth considering. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut said: > Martijn van Oosterhout wrote: >> Incidently, IIRC the default behaviour on conflict is a shift anyway, >> so that what the patch already does anyway. >> >> So we get: >> >> CREATE VIEW foo AS SELECT expr :: TIME WITH TIME ZONE <-- OK >> CREATE VIEW foo AS SELECT expr :: TIME WITH CHECK OPTION <-- > >> parse error CREATE VIEW foo AS SELECT (expr :: TIME) WITH CHECK OPTION >> <-- OK > > Yes, that's really the fundamental problem if you let shift/reduce > conflicts stand: the parser will behave weirdly in the conflict cases. > > There is a seemingly little known option in bison named %glr-parser, > which when turned on parses all of theses cases correctly. Maybe that > is worth considering. Interesting. Unfortunately, the manual says: "The GLR parsers require a compiler for ISO C89 or later. In addition, they use the inline keyword, which is not C89, but is C99 and is a common extension in pre-C99 compilers. It is up to the user of these parsers to handle portability issues." Do we want such a restriction? cheers andrew
Andrew Dunstan wrote: > "The GLR parsers require a compiler for ISO C89 or later. In > addition, they use the inline keyword, which is not C89, but is C99 > and is a common extension in pre-C99 compilers. It is up to the user > of these parsers to handle portability issues." We already use inline, or handle its nonexistence, respectively. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes:
> Andrew Dunstan wrote:
>> "The GLR parsers require a compiler for ISO C89 or later. In
>> addition, they use the inline keyword, which is not C89, but is C99
>> and is a common extension in pre-C99 compilers. It is up to the user
>> of these parsers to handle portability issues."
> We already use inline, or handle its nonexistence, respectively.
Yeah, I don't see anything in that statement that we don't assume
already. The interesting question to me is how much different is
GLR from garden-variety bison; in particular, what's the parser
performance like?
regards, tom lane
Tom Lane said: > Peter Eisentraut <peter_e@gmx.net> writes: >> Andrew Dunstan wrote: >>> "The GLR parsers require a compiler for ISO C89 or later. In >>> addition, they use the inline keyword, which is not C89, but is C99 >>> and is a common extension in pre-C99 compilers. It is up to the user >>> of these parsers to handle portability issues." > >> We already use inline, or handle its nonexistence, respectively. > > Yeah, I don't see anything in that statement that we don't assume > already. The interesting question to me is how much different is > GLR from garden-variety bison; in particular, what's the parser > performance like? > As I understand it, it runs one parser pretty much like the standard LALR(1) case, until it finds an ambiguity (shift/reduce or reduce/reduce) at which stage it clones the parser to take parallel paths, working in lockstep, and storing up semantic actions. When one of the clones encounters an error it goes away, and the surviving clone takes its stored semantic actions. If that's true, then probably the only significant performance hit is in cases of ambiguity, and we would only have a handful of those, each lasting for one token, so the performance hit should be very small. We'd have to test it, of course ;-) cheers andrew
"Andrew Dunstan" <andrew@dunslane.net> writes:
> As I understand it, it runs one parser pretty much like the standard LALR(1)
> case, until it finds an ambiguity (shift/reduce or reduce/reduce) at which
> stage it clones the parser to take parallel paths, working in lockstep, and
> storing up semantic actions. When one of the clones encounters an error it
> goes away, and the surviving clone takes its stored semantic actions. If
> that's true, then probably the only significant performance hit is in cases
> of ambiguity, and we would only have a handful of those, each lasting for
> one token, so the performance hit should be very small. We'd have to test
> it, of course ;-)
Yeah, I just read the same in the bison manual. The thing that's
bothering me is that a GLR parser would hide that ambiguity from you,
and thus changes in the grammar might cause us to incur performance
hits without realizing it. The manual indicates that the performance
is pretty awful whenever an ambiguity does occur.
regards, tom lane
Tom Lane wrote: > Yeah, I just read the same in the bison manual. The thing that's > bothering me is that a GLR parser would hide that ambiguity from you, It doesn't really hide it. You still get the "N shift/reduce conflicts" warnings from bison. You just know that they are being handled. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes: > Tom Lane wrote: >> Yeah, I just read the same in the bison manual. The thing that's >> bothering me is that a GLR parser would hide that ambiguity from you, > It doesn't really hide it. You still get the "N shift/reduce conflicts" > warnings from bison. You just know that they are being handled. Well, that has the same problem that we've raised every other time someone has said "I don't want to fix the grammar to not have any conflicts". If bison only tells you "there were N conflicts", how do you know these are the same N conflicts you had yesterday? In a grammar that we hack around as much as we do with Postgres, I really don't think that's acceptable. I think that by far the most reliable solution is to put back the "filter" yylex function that I removed a couple months ago: http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/parser/parser.c.diff?r1=1.64&r2=1.65 We can use the same technique that we used for UNION JOIN, but instead join, say, WITH and TIME into one token and make the time datatype productions look for "TIME WITHTIME ZONE" and so on. (I propose this rather than putting the ugliness into WITH CHECK OPTION, because this way we should only need one merged token and thus only one case to check in the filter function; AFAICS we'd need three cases if we merge tokens on that end of it.) I'm not sure we can just revert the above-mentioned patch, because it had some interactions with a later patch to use %option prefix. Shouldn't be too hard to fix though. I'll put together a proposed patch. regards, tom lane
I wrote:
> We can use the same technique that we used for UNION JOIN, but instead
> join, say, WITH and TIME into one token and make the time datatype
> productions look for "TIME WITHTIME ZONE" and so on. (I propose this
> rather than putting the ugliness into WITH CHECK OPTION, because this
> way we should only need one merged token and thus only one case to
> check in the filter function; AFAICS we'd need three cases if we
> merge tokens on that end of it.)
On investigation that turns out to have been a bad idea: if we do it
that way, it becomes necessary to promote WITH to a fully reserved word.
The counterexample is
CREATE VIEW v AS SELECT * FROM foo WITH ...
Is WITH an alias for foo (with no AS), or is it the start of a WITH
CHECK OPTION? No way to tell without lookahead.
While I don't think that making WITH a fully reserved word would cause
any great damage, I'm unwilling to do it just to save a couple of lines
of code. Accordingly, I propose the attached patch. This reinstates
the filter yylex function formerly used for UNION JOIN (in a slightly
cleaner fashion than it was previously done) and parses WITH CHECK
OPTION without any bison complaints, and with no new reserved words.
If no objections, I'll go ahead and apply this, and Peter can get on
with making the stub productions do something useful.
regards, tom lane
Index: src/backend/parser/Makefile
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/Makefile,v
retrieving revision 1.43
diff -c -r1.43 Makefile
*** src/backend/parser/Makefile 7 Mar 2006 01:00:17 -0000 1.43
--- src/backend/parser/Makefile 27 May 2006 02:39:51 -0000
***************
*** 57,63 ****
# Force these dependencies to be known even without dependency info built:
! gram.o keywords.o: $(srcdir)/parse.h
# gram.c, parse.h, and scan.c are in the distribution tarball, so they
--- 57,63 ----
# Force these dependencies to be known even without dependency info built:
! gram.o keywords.o parser.o: $(srcdir)/parse.h
# gram.c, parse.h, and scan.c are in the distribution tarball, so they
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.544
diff -c -r2.544 gram.y
*** src/backend/parser/gram.y 30 Apr 2006 18:30:39 -0000 2.544
--- src/backend/parser/gram.y 27 May 2006 02:39:52 -0000
***************
*** 70,75 ****
--- 70,81 ----
(Current) = (Rhs)[0]; \
} while (0)
+ /*
+ * The %name-prefix option below will make bison call base_yylex, but we
+ * really want it to call filtered_base_yylex (see parser.c).
+ */
+ #define base_yylex filtered_base_yylex
+
extern List *parsetree; /* final parse result is delivered here */
static bool QueryIsRule = FALSE;
***************
*** 339,344 ****
--- 345,351 ----
%type <list> constraints_set_list
%type <boolean> constraints_set_mode
%type <str> OptTableSpace OptConsTableSpace OptTableSpaceOwner
+ %type <list> opt_check_option
/*
***************
*** 356,362 ****
BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT
BOOLEAN_P BOTH BY
! CACHE CALLED CASCADE CASE CAST CHAIN CHAR_P
CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE
CLUSTER COALESCE COLLATE COLUMN COMMENT COMMIT
COMMITTED CONNECTION CONSTRAINT CONSTRAINTS CONVERSION_P CONVERT COPY CREATE CREATEDB
--- 363,369 ----
BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT
BOOLEAN_P BOTH BY
! CACHE CALLED CASCADE CASCADED CASE CAST CHAIN CHAR_P
CHARACTER CHARACTERISTICS CHECK CHECKPOINT CLASS CLOSE
CLUSTER COALESCE COLLATE COLUMN COMMENT COMMIT
COMMITTED CONNECTION CONSTRAINT CONSTRAINTS CONVERSION_P CONVERT COPY CREATE CREATEDB
***************
*** 431,436 ****
--- 438,449 ----
ZONE
+ /* The grammar thinks these are keywords, but they are not in the keywords.c
+ * list and so can never be entered directly. The filter in parser.c
+ * creates these tokens when required.
+ */
+ %token WITH_CASCADED WITH_LOCAL WITH_CHECK
+
/* Special token types, not actually keywords - see the "lex" file */
%token <str> IDENT FCONST SCONST BCONST XCONST Op
%token <ival> ICONST PARAM
***************
*** 4618,4629 ****
/*****************************************************************************
*
* QUERY:
! * CREATE [ OR REPLACE ] [ TEMP ] VIEW <viewname> '('target-list ')' AS <query>
*
*****************************************************************************/
ViewStmt: CREATE OptTemp VIEW qualified_name opt_column_list
! AS SelectStmt
{
ViewStmt *n = makeNode(ViewStmt);
n->replace = false;
--- 4631,4643 ----
/*****************************************************************************
*
* QUERY:
! * CREATE [ OR REPLACE ] [ TEMP ] VIEW <viewname> '('target-list ')'
! * AS <query> [ WITH [ CASCADED | LOCAL ] CHECK OPTION ]
*
*****************************************************************************/
ViewStmt: CREATE OptTemp VIEW qualified_name opt_column_list
! AS SelectStmt opt_check_option
{
ViewStmt *n = makeNode(ViewStmt);
n->replace = false;
***************
*** 4634,4640 ****
$$ = (Node *) n;
}
| CREATE OR REPLACE OptTemp VIEW qualified_name opt_column_list
! AS SelectStmt
{
ViewStmt *n = makeNode(ViewStmt);
n->replace = true;
--- 4648,4654 ----
$$ = (Node *) n;
}
| CREATE OR REPLACE OptTemp VIEW qualified_name opt_column_list
! AS SelectStmt opt_check_option
{
ViewStmt *n = makeNode(ViewStmt);
n->replace = true;
***************
*** 4646,4651 ****
--- 4660,4691 ----
}
;
+ /*
+ * We use merged tokens here to avoid creating shift/reduce conflicts against
+ * a whole lot of other uses of WITH.
+ */
+ opt_check_option:
+ WITH_CHECK OPTION
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("WITH CHECK OPTION is not implemented")));
+ }
+ | WITH_CASCADED CHECK OPTION
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("WITH CHECK OPTION is not implemented")));
+ }
+ | WITH_LOCAL CHECK OPTION
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("WITH CHECK OPTION is not implemented")));
+ }
+ | /* EMPTY */ { $$ = NIL; }
+ ;
+
/*****************************************************************************
*
* QUERY:
***************
*** 8319,8324 ****
--- 8359,8365 ----
| CACHE
| CALLED
| CASCADE
+ | CASCADED
| CHAIN
| CHARACTERISTICS
| CHECKPOINT
***************
*** 9139,9142 ****
--- 9180,9189 ----
}
}
+ /*
+ * Must undefine base_yylex before including scan.c, since we want it
+ * to create the function base_yylex not filtered_base_yylex.
+ */
+ #undef base_yylex
+
#include "scan.c"
Index: src/backend/parser/parser.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parser.c,v
retrieving revision 1.65
diff -c -r1.65 parser.c
*** src/backend/parser/parser.c 7 Mar 2006 01:00:17 -0000 1.65
--- src/backend/parser/parser.c 27 May 2006 02:39:52 -0000
***************
*** 22,32 ****
--- 22,36 ----
#include "postgres.h"
#include "parser/gramparse.h"
+ #include "parser/parse.h"
#include "parser/parser.h"
List *parsetree; /* result of parsing is left here */
+ static int lookahead_token; /* one-token lookahead */
+ static bool have_lookahead; /* lookahead_token set? */
+
/*
* raw_parser
***************
*** 40,45 ****
--- 44,50 ----
int yyresult;
parsetree = NIL; /* in case grammar forgets to set it */
+ have_lookahead = false;
scanner_init(str);
parser_init();
***************
*** 53,55 ****
--- 58,120 ----
return parsetree;
}
+
+
+ /*
+ * Intermediate filter between parser and base lexer (base_yylex in scan.l).
+ *
+ * The filter is needed because in some cases the standard SQL grammar
+ * requires more than one token lookahead. We reduce these cases to one-token
+ * lookahead by combining tokens here, in order to keep the grammar LALR(1).
+ *
+ * Using a filter is simpler than trying to recognize multiword tokens
+ * directly in scan.l, because we'd have to allow for comments between the
+ * words. Furthermore it's not clear how to do it without re-introducing
+ * scanner backtrack, which would cost more performance than this filter
+ * layer does.
+ */
+ int
+ filtered_base_yylex(void)
+ {
+ int cur_token;
+
+ /* Get next token --- we might already have it */
+ if (have_lookahead)
+ {
+ cur_token = lookahead_token;
+ have_lookahead = false;
+ }
+ else
+ cur_token = base_yylex();
+
+ /* Do we need to look ahead for a possible multiword token? */
+ switch (cur_token)
+ {
+ case WITH:
+ /*
+ * WITH CASCADED, LOCAL, or CHECK must be reduced to one token
+ */
+ lookahead_token = base_yylex();
+ switch (lookahead_token)
+ {
+ case CASCADED:
+ cur_token = WITH_CASCADED;
+ break;
+ case LOCAL:
+ cur_token = WITH_LOCAL;
+ break;
+ case CHECK:
+ cur_token = WITH_CHECK;
+ break;
+ default:
+ have_lookahead = true;
+ break;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return cur_token;
+ }
Index: src/include/parser/gramparse.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/parser/gramparse.h,v
retrieving revision 1.36
diff -c -r1.36 gramparse.h
*** src/include/parser/gramparse.h 21 May 2006 20:10:42 -0000 1.36
--- src/include/parser/gramparse.h 27 May 2006 02:39:52 -0000
***************
*** 40,45 ****
--- 40,48 ----
extern bool standard_conforming_strings;
+ /* from parser.c */
+ extern int filtered_base_yylex(void);
+
/* from scan.l */
extern void scanner_init(const char *str);
extern void scanner_finish(void);
Ühel kenal päeval, R, 2006-05-26 kell 22:50, kirjutas Tom Lane: > I wrote: > > We can use the same technique that we used for UNION JOIN, but instead > > join, say, WITH and TIME into one token and make the time datatype > > productions look for "TIME WITHTIME ZONE" and so on. (I propose this > > rather than putting the ugliness into WITH CHECK OPTION, because this > > way we should only need one merged token and thus only one case to > > check in the filter function; AFAICS we'd need three cases if we > > merge tokens on that end of it.) > > On investigation that turns out to have been a bad idea: if we do it > that way, it becomes necessary to promote WITH to a fully reserved word. > The counterexample is > > CREATE VIEW v AS SELECT * FROM foo WITH ... > > Is WITH an alias for foo (with no AS), or is it the start of a WITH > CHECK OPTION? No way to tell without lookahead. > > While I don't think that making WITH a fully reserved word would cause > any great damage, I'm unwilling to do it just to save a couple of lines > of code. I think we should go on and do promote WITH to a reserved keyword now because eventually we have to do it anyway. It is needed for recursive queries as well. I don't pretend to be an expert bison coder, but I was unable to define a grammar for SQL-standard recursive queries without making WITH a reserved keyword. -- ---------------- Hannu Krosing Database Architect Skype Technologies OÜ Akadeemia tee 21 F, Tallinn, 12618, Estonia Skype me: callto:hkrosing Get Skype for free: http://www.skype.com
Hannu Krosing <hannu@skype.net> writes:
> I think we should go on and do promote WITH to a reserved keyword now
> because eventually we have to do it anyway.
> It is needed for recursive queries as well.
I'm unconvinced. Recursive queries have WITH at the front, not the
back, so the parsing issues are entirely different.
If we do find that, we can easily adjust this code to simplify the
filter function at that time. But I don't agree with reserving words
just because we might need them for patches that don't exist yet.
regards, tom lane
> > While I don't think that making WITH a fully reserved word would cause > > any great damage, I'm unwilling to do it just to save a couple of lines > > of code. > > I think we should go on and do promote WITH to a reserved keyword now Oracle, MS-SQL, DB2, MySQL and Informix also have WITH reserved, so it would imho be ok to do it if it simplifies code. Andreas