Thread: adding stuff to parser, question
Hey folks,
I am trying to add "GRANT SELECT ON ALL TABLES" to postgres, as it has
been quite few times now - where I had to write a procedure for that.
I kind of looked around, and more or less know how to approach it. But
I am stuck on parser :), yes - parser.
Can someone walk me through adding new rules to parser ?
so far I have this:
Index: parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.656
diff -u -r2.656 gram.y
--- parser/gram.y 22 Jan 2009 20:16:05 -0000 2.656
+++ parser/gram.y 31 Jan 2009 16:44:57 -0000
@@ -494,7 +494,7 @@ STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P SUBSTRING
SUPERUSER_P SYMMETRIC SYSID SYSTEM_P
- TABLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN TIME TIMESTAMP
+ TABLE TABLES TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN TIME
TIMESTAMP TO TRAILING TRANSACTION TREAT TRIGGER TRIM TRUE_P TRUNCATE TRUSTED TYPE_P
@@ -4301,6 +4301,13 @@ n->objs = $2; $$ = n; }
+ | ALL TABLES
+ {
+ PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
+ n->objtype = ACL_OBJECT_RELATION;
+ n->objs = NULL;
+ $$ = n;
+ } | SEQUENCE qualified_name_list { PrivTarget *n =
(PrivTarget*) palloc(sizeof(PrivTarget));
Index: parser/keywords.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.209
diff -u -r1.209 keywords.c
--- parser/keywords.c 1 Jan 2009 17:23:45 -0000 1.209
+++ parser/keywords.c 31 Jan 2009 16:44:57 -0000
@@ -373,6 +373,7 @@ {"sysid", SYSID, UNRESERVED_KEYWORD}, {"system", SYSTEM_P, UNRESERVED_KEYWORD},
{"table",TABLE, RESERVED_KEYWORD},
+ {"table", TABLES, RESERVED_KEYWORD}, {"tablespace", TABLESPACE, UNRESERVED_KEYWORD}, {"temp", TEMP,
UNRESERVED_KEYWORD}, {"template", TEMPLATE, UNRESERVED_KEYWORD},
But that seems to be not nearly enough, for psql to recognize "GRANT
SELECT ON ALL TABLES TO foo".
Please help me out, with stuff I am missing here. I never had any
expierence with bison, or any other lexical parsers for that matter.
Thanks. :)
On 31 Jan 2009, at 16:46, Grzegorz Jaskiewicz wrote:
>
> + {"table", TABLES, RESERVED_KEYWORD},
Gaaah, a typo...
:(
(another useless post to -hackers, by myself).
Grzegorz Jaskiewicz <gj@pointblue.com.pl> writes:
You're going to kick yourself, but:
> {"table", TABLE, RESERVED_KEYWORD},
> + {"table", TABLES, RESERVED_KEYWORD},
^
I don't see any reason offhand why it should have to be a reserved word
though. You should be able to make it an UNRESERVED_KEYWORD. Oh, and you'll
want to add it to the list of tokens in unreserved_keyword in gram.y as well.
-- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about
EnterpriseDB'sPostgreSQL training!
On Sat, Jan 31, 2009 at 04:46:26PM +0000, Grzegorz Jaskiewicz wrote: > Hey folks, > > I am trying to add "GRANT SELECT ON ALL TABLES" to postgres, Is this part of the SQL:2008? If not, is there something else that is? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On 31 Jan 2009, at 17:17, Gregory Stark wrote:
> Grzegorz Jaskiewicz <gj@pointblue.com.pl> writes:
>
> You're going to kick yourself, but:
>
>
>> {"table", TABLE, RESERVED_KEYWORD},
>> + {"table", TABLES, RESERVED_KEYWORD},
>
> ^
>
> I don't see any reason offhand why it should have to be a reserved
> word
> though. You should be able to make it an UNRESERVED_KEYWORD. Oh, and
> you'll
> want to add it to the list of tokens in unreserved_keyword in gram.y
> as well.
I am really novice with parsers here, so - I felt like I have to do
it, in order to make it work. It just wasn't working without that bit
in keywords.c. I shall try your way, thanks :)
from http://wiki.postgresql.org/wiki/Todo: [E] Allow GRANT/REVOKE permissions to be applied to all schema objects with one command The proposed syntax is: GRANT SELECT ON ALL TABLES IN public TO phpuser; GRANT SELECT ON NEW TABLES IN public TO phpuser;
Attachment
On Sat, Jan 31, 2009 at 05:24:15PM +0000, Grzegorz Jaskiewicz wrote: > from http://wiki.postgresql.org/wiki/Todo: I see the TODO item, but I don't see anything in the SQL standard, which I think is something we should explore before making a potentially incompatible change. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Grzegorz Jaskiewicz wrote: > from http://wiki.postgresql.org/wiki/Todo: > > [E] > ------------------------------------------------------------------------ > > Allow GRANT/REVOKE permissions to be applied to all schema objects > with one command > The proposed syntax is: GRANT SELECT ON ALL TABLES IN public TO > phpuser; GRANT SELECT ON NEW TABLES IN public TO phpuser; > > > > ------------------------------------------------------------------------ > > But the syntax you posted does not do this at all. Where does it restrict the grant to a single schema, like the syntax above? cheers andrew
On 31 Jan 2009, at 17:30, Andrew Dunstan wrote: > But the syntax you posted does not do this at all. Where does it > restrict the grant to a single schema, like the syntax above? I am just starting the attempt here, obviously since I admit that my parser skills are next to none - I didn't address such issue. So far, testing this change - I can do issue "GRANT SELECT ON ALL TABLES TO xxx", and it parses well.
On 31 Jan 2009, at 17:28, David Fetter wrote: > On Sat, Jan 31, 2009 at 05:24:15PM +0000, Grzegorz Jaskiewicz wrote: >> from http://wiki.postgresql.org/wiki/Todo: > > I see the TODO item, but I don't see anything in the SQL standard, > which I think is something we should explore before making a > potentially incompatible change. Personally, I don't think we should follow SQL 2008 to the letter. I am sure, many ppl miss that ability - I know I do, so I wanted to implement that right in the core. Wether SQL standard has something of same functionality but different syntax or not, well - I would love to know too - but I never read SQL standard - to be honest.
On Sat, Jan 31, 2009 at 05:40:57PM +0000, Grzegorz Jaskiewicz wrote:
> On 31 Jan 2009, at 17:30, Andrew Dunstan wrote:
>> But the syntax you posted does not do this at all. Where does it
>> restrict the grant to a single schema, like the syntax above?
> I am just starting the attempt here, obviously since I admit that my
> parser skills are next to none - I didn't address such issue.
> So far, testing this change - I can do issue "GRANT SELECT ON ALL TABLES
> TO xxx", and it parses well.
Your desire to figure out how to add stuff to the parser is certainly
commendable, and you might consider continuing down the road you've started on
for that purpose alone. But the desire of others on this list to remain close
to the SQL standard is equally commendable, and unlikely to go away :) If your
main purpose is to get the feature implemented, whether or not you learn how
to add new syntax, you might consider writing a function instead. This
function might take parameters such as the privilege to grant and the user to
grant it to, and be called something like this:
SELECT my_grant_function('someuser', 'someprivilege');
This would do what you need, and in no way conflict with the standard if one
day it covers the feature in question.
- Josh / eggyknap
On 1 Feb 2009, at 00:05, Joshua Tolley wrote:
>
> to add new syntax, you might consider writing a function instead. This
> function might take parameters such as the privilege to grant and
> the user to
> grant it to, and be called something like this:
>
> SELECT my_grant_function('someuser', 'someprivilege');
Well, if you read my first post - I did wrote such a function, and it
works just fine. But still - since that was in TODO, I figured I might
give it a go.
On 31 Jan 2009, at 17:17, Gregory Stark wrote: > > I don't see any reason offhand why it should have to be a reserved > word > though. You should be able to make it an UNRESERVED_KEYWORD. Oh, and > you'll > want to add it to the list of tokens in unreserved_keyword in gram.y > as well. removing it from keywords.c and adding it to unserserved_keywords crowd didn't make it... so I'll stick with keywords.c for timebeing. So far I got mostly critique here, even tho - I haven't started much, which is quite sad in a way - because it is not very pro-creative, but I'll still continue on with the patch - whatever the outcome. ta.
On 31 Jan 2009, at 17:22, David Fetter wrote: > > Is this part of the SQL:2008? If not, is there something else that > is? As far as I can see in the 'free' draft, no. Which is quite funny, cos 'DATABASE' keyword isn't there too.. Anyway. Looks like m$'s sybase clone got it, so I figure - at least some ppl figured it would be a nice feature ;) Can someone lead me into, how should I grab all Oid's for tables (in all user defined schemas, public. and others in db - except for system ones) in /src/backend/catalog/namespace.c please ? (which would probably be the best place to do it.) so far I figured, that adding another ACL_OBJECT define would be the best option, instead of passing NIL in objnames, for which there's an assert at begin of objectNamesToOids(). Reason I am asking about the backend/catalog approach, is because of all structures, and existing code (which I only got to go through Today for first time), I don't see any existing example, that would enumerate 'everything' in specific category. Thanks.
Grzegorz Jaskiewicz wrote: > > On 31 Jan 2009, at 17:22, David Fetter wrote: >> >> Is this part of the SQL:2008? If not, is there something else that >> is? > > As far as I can see in the 'free' draft, no. Which is quite funny, cos > 'DATABASE' keyword isn't there too.. > Anyway. Looks like m$'s sybase clone got it, so I figure - at least > some ppl figured it would be a nice feature ;) The standard doesn't have the concept of a database. In Postgres, a database is essentially the same as the standard's concept of a catalog. cheers andrew
On 1 Feb 2009, at 10:25, Gregory Stark wrote: > > I'm sorry if I was unclear. It needs to be in keywords.c but can > probably be > marked as UNRESERVED_KEYWORD there rather than RESERVED_KEYWORD. > > In other words there are two places where you have to indicate > whether it's > reserved or not, keywords.c and gram.y. > Ok, ACKed. >> So far I got mostly critique here, even tho - I haven't started >> much, which is >> quite sad in a way - because it is not very pro-creative, but I'll >> still >> continue on with the patch - whatever the outcome. > > Any change to the grammar meets the question of whether it conflicts > with the > standard. That's just the way it is and doesn't reflect on you or > your work. Sure, there's much broad problem with it too. Wether it should grant/revoke SELECT to all user defined tables? In all schemas, except for pg_catalog and information schema (the later, I believe is already SELECT granted for all users). Hence my question yesterday, how can I make sure I got all of these oids. I was suggested to use SearchSysCache* stuff to grab oids, but honestly, I wouldn't mind to get some directions on that from you guys here. thanks.
Grzegorz Jaskiewicz <gj@pointblue.com.pl> writes: > On 31 Jan 2009, at 17:17, Gregory Stark wrote: >> >> I don't see any reason offhand why it should have to be a reserved word >> though. You should be able to make it an UNRESERVED_KEYWORD. Oh, and you'll >> want to add it to the list of tokens in unreserved_keyword in gram.y as >> well. > > removing it from keywords.c and adding it to unserserved_keywords crowd didn't > make it... so I'll stick with keywords.c for timebeing. I'm sorry if I was unclear. It needs to be in keywords.c but can probably be marked as UNRESERVED_KEYWORD there rather than RESERVED_KEYWORD. It has to be in the grammar rule for the corresponding production, either reserved_keyword or unreserved_keyword. In other words there are two places where you have to indicate whether it's reserved or not, keywords.c and gram.y. > So far I got mostly critique here, even tho - I haven't started much, which is > quite sad in a way - because it is not very pro-creative, but I'll still > continue on with the patch - whatever the outcome. Any change to the grammar meets the question of whether it conflicts with the standard. That's just the way it is and doesn't reflect on you or your work. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support!
On Sun, Feb 01, 2009 at 10:25:29AM +0000, Gregory Stark wrote: > > removing it from keywords.c and adding it to unserserved_keywords crowd didn't > > make it... so I'll stick with keywords.c for timebeing. > > I'm sorry if I was unclear. It needs to be in keywords.c but can probably be > marked as UNRESERVED_KEYWORD there rather than RESERVED_KEYWORD. Quite frankly, the grammer is not that hard part. If you can get it to work with any grammer at all, some yacc guru can fix it to match any grammer anyone wants in a very short time. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines.
On Sun, Feb 01, 2009 at 12:12:47AM +0000, Grzegorz Jaskiewicz wrote:
> On 1 Feb 2009, at 00:05, Joshua Tolley wrote:
>> to add new syntax, you might consider writing a function instead. This
>> function might take parameters such as the privilege to grant and the
>> user to
>> grant it to, and be called something like this:
>>
>> SELECT my_grant_function('someuser', 'someprivilege');
>
> Well, if you read my first post - I did wrote such a function, and it
> works just fine. But still - since that was in TODO, I figured I might
> give it a go.
Ah, that you did. Sorry. :)
- Josh
On Saturday 31 January 2009 19:30:36 Andrew Dunstan wrote: > > ------------------------------------------------------------------------ > > > > Allow GRANT/REVOKE permissions to be applied to all schema objects > > with one command > > The proposed syntax is: GRANT SELECT ON ALL TABLES IN public TO > > phpuser; GRANT SELECT ON NEW TABLES IN public TO phpuser; > > > > > > > > ------------------------------------------------------------------------ > > But the syntax you posted does not do this at all. Where does it > restrict the grant to a single schema, like the syntax above? Since any kind of implementation is going to have to scan and filter pg_class (and possibly pg_namespace), I would consider going one step further and allowing some kind of wildcard mechanism. This could later be extended to lots of other useful places, such as, drop all tables like 'test%'.