Abhijit Menon-Sen wrote:
I have not yet been able to do a complete review of this patch, but I am
posting this because I'll be travelling for a week starting tomorrow. My
comments are based mostly on reading the patch, and not on any intensive
testing of the feature. I have left the patch status unchanged at "needs
review", although I think it's close to "ready for committer".
Thanks for your review.
1. The patch did apply to HEAD and build cleanly, but there are now a couple of minor (documentation) conflicts. (Sorry, I would have fixed them and reposted a patch, but I'm running out of time right now.)
I fixed those conflicts in attached patch.
*** a/doc/src/sgml/ref/grant.sgml
--- b/doc/src/sgml/ref/grant.sgml
[...]
<para>
+ There is also the possibility of granting permissions to all objects of
+ given type inside one or multiple schemas. This functionality is supported
+ for tables, views, sequences and functions and can done by using
+ ALL {TABLES|SEQUENCES|FUNCTIONS} IN SCHEMA schemaname syntax in place
+ of object name.
+ </para>
+
+ <para>
2. Here I suggest the following wording:
<para> You can also grant permissions on all tables, sequences, or functions that currently exist within a given schema by specifying "ALL {TABLES|SEQUENCES|FUNCTIONS} IN SCHEMA schemaname" in place of an object name. </para>
3. I believe MySQL's "grant all privileges on foo.* to someone" grants privileges on all existing objects in foo _but also_ on any objects that may be created later. This patch only gives you a way to grant privileges only on the objects currently within a schema. I strongly prefer this behaviour myself, but I do think the documentation needs a brief mention of this fact, to avoid surprising people. That's why I added "that currently exist" to (2), above. Maybe another sentence that specifically says that objects created later are unaffected is in order. I'm not sure.
I'll leave the exact wording to commiter, but in the attached patch I changed it to say "all existing objects" instead of "all objects".
Except for above two changes and the fact that it's against current head, the patch is exactly the same.
Thanks again.
--
Regards
Petr Jelinek (PJMODOS)
