Re: [HACKERS] [BUGS] COPY .... (FORMAT binary) syntax doesn't work
On 27 May 2013 15:31, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 26 May 2013 17:10, Tom Lane t...@sss.pgh.pa.us wrote: More readable would be to invent an intermediate nonterminal falling between ColId and ColLabel, whose expansion would be IDENT | unreserved_keyword | col_name_keyword | type_func_name_keyword, and then replace ColId_or_Sconst with whatever-we-call-that_or_Sconst. Any thoughts about a name for that new nonterminal? Do you think complicating the parser in that way is worth the trouble for this case? Could that slow down parsing? It makes the grammar tables a bit larger (1% or so IIRC). There would be some distributed penalty for that, but probably not much. Of course there's always the slippery-slope argument about that. We don't actually have to fix it; clearly not too many people are bothered, since no complaints in 3 years. Documenting 'binary' seems better. Well, my thought is there are other cases. For instance: regression=# create role binary; ERROR: syntax error at or near binary LINE 1: create role binary; ^ regression=# create user cross; ERROR: syntax error at or near cross LINE 1: create user cross; ^ If we don't have to treat type_func_name_keywords as reserved in these situations, shouldn't we avoid doing so? Seems reasonable argument, so +1. Sorry for delayed reply. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [HACKERS] [BUGS] COPY .... (FORMAT binary) syntax doesn't work
On Mon, May 27, 2013 at 10:31 AM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 26 May 2013 17:10, Tom Lane t...@sss.pgh.pa.us wrote: More readable would be to invent an intermediate nonterminal falling between ColId and ColLabel, whose expansion would be IDENT | unreserved_keyword | col_name_keyword | type_func_name_keyword, and then replace ColId_or_Sconst with whatever-we-call-that_or_Sconst. Any thoughts about a name for that new nonterminal? Do you think complicating the parser in that way is worth the trouble for this case? Could that slow down parsing? It makes the grammar tables a bit larger (1% or so IIRC). There would be some distributed penalty for that, but probably not much. Of course there's always the slippery-slope argument about that. We don't actually have to fix it; clearly not too many people are bothered, since no complaints in 3 years. Documenting 'binary' seems better. Well, my thought is there are other cases. For instance: regression=# create role binary; ERROR: syntax error at or near binary LINE 1: create role binary; ^ regression=# create user cross; ERROR: syntax error at or near cross LINE 1: create user cross; ^ If we don't have to treat type_func_name_keywords as reserved in these situations, shouldn't we avoid doing so? I am almost always in favor of making more things less reserved, so +1 from me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [HACKERS] [BUGS] COPY .... (FORMAT binary) syntax doesn't work
On 26 May 2013 17:10, Tom Lane t...@sss.pgh.pa.us wrote: Heikki Linnakangas hlinnakan...@vmware.com writes: More readable would be to invent an intermediate nonterminal falling between ColId and ColLabel, whose expansion would be IDENT | unreserved_keyword | col_name_keyword | type_func_name_keyword, and then replace ColId_or_Sconst with whatever-we-call-that_or_Sconst. Any thoughts about a name for that new nonterminal? Do you think complicating the parser in that way is worth the trouble for this case? Could that slow down parsing? We don't actually have to fix it; clearly not too many people are bothered, since no complaints in 3 years. Documenting 'binary' seems better. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [HACKERS] [BUGS] COPY .... (FORMAT binary) syntax doesn't work
Simon Riggs si...@2ndquadrant.com writes: On 26 May 2013 17:10, Tom Lane t...@sss.pgh.pa.us wrote: More readable would be to invent an intermediate nonterminal falling between ColId and ColLabel, whose expansion would be IDENT | unreserved_keyword | col_name_keyword | type_func_name_keyword, and then replace ColId_or_Sconst with whatever-we-call-that_or_Sconst. Any thoughts about a name for that new nonterminal? Do you think complicating the parser in that way is worth the trouble for this case? Could that slow down parsing? It makes the grammar tables a bit larger (1% or so IIRC). There would be some distributed penalty for that, but probably not much. Of course there's always the slippery-slope argument about that. We don't actually have to fix it; clearly not too many people are bothered, since no complaints in 3 years. Documenting 'binary' seems better. Well, my thought is there are other cases. For instance: regression=# create role binary; ERROR: syntax error at or near binary LINE 1: create role binary; ^ regression=# create user cross; ERROR: syntax error at or near cross LINE 1: create user cross; ^ If we don't have to treat type_func_name_keywords as reserved in these situations, shouldn't we avoid doing so? regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [HACKERS] [BUGS] COPY .... (FORMAT binary) syntax doesn't work
Heikki Linnakangas hlinnakan...@vmware.com writes:
On 26.05.2013 04:31, Simon Riggs wrote:
This new format does not [work:]
COPY pgbench_accounts FROM '/tmp/acc' (FORMAT BINARY);
ERROR: syntax error at or near BINARY at character 47
This seems to work:
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2528,3 +2528,7 @@ copy_generic_opt_elem:
{
$$ = makeDefElem($1, $2);
}
+ | ColLabel BINARY
+ {
+ $$ = makeDefElem($1, (Node *)
makeString(binary));
+ }
That only fixes things for the specific case of FORMAT BINARY. I think
it would be better to generalize ColId_or_Sconst. This one-liner works,
but it's pretty ugly:
*** gram.y~ Sun May 26 11:58:42 2013
--- gram.y Sun May 26 12:00:03 2013
*** opt_encoding:
*** 1548,1553
--- 1548,1554
ColId_or_Sconst:
ColId
{ $$ = $1; }
+ | type_func_name_keyword
{ $$ = pstrdup($1); }
| Sconst
{ $$ = $1; }
;
More readable would be to invent an intermediate nonterminal falling
between ColId and ColLabel, whose expansion would be IDENT |
unreserved_keyword | col_name_keyword | type_func_name_keyword, and
then replace ColId_or_Sconst with whatever-we-call-that_or_Sconst.
Any thoughts about a name for that new nonterminal?
BTW, I tried just replacing ColId with ColLabel here, and that *almost*
works, but there are some places where we can see either ColId_or_Sconst
or DEFAULT. I don't know of any sane way to express all reserved
keywords except DEFAULT to bison, so the best we can realistically do
is to accept all not-fully-reserved keywords in these places.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
