Received: from maia.hub.org (unknown [200.46.204.183])
	by mail.postgresql.org (Postfix) with ESMTP id CCC99633E10
	for <pgsql-hackers-postgresql.org@mail.postgresql.org>;
	Fri, 17 Jul 2009 06:55:07 -0300 (ADT)
Received: from mail.postgresql.org ([200.46.204.86])
	by maia.hub.org (mx1.hub.org [200.46.204.183]) (amavisd-maia,
	port 10024) with ESMTP id 95579-04
	for <pgsql-hackers-postgresql.org@mail.postgresql.org>;
	Fri, 17 Jul 2009 06:54:57 -0300 (ADT)
X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6
Received: from exprod7og125.obsmtp.com (exprod7og125.obsmtp.com [64.18.2.28])
	by mail.postgresql.org (Postfix) with SMTP id 0C86863363A
	for <pgsql-hackers@postgresql.org>;
	Fri, 17 Jul 2009 06:54:56 -0300 (ADT)
Received: from source ([209.85.222.191]) by exprod7ob125.postini.com
	([64.18.6.12]) with SMTP
	ID DSNKSmBKb4NXllSxyIT++4ESvUqVnH6pe1ad@postini.com;
	Fri, 17 Jul 2009 02:54:56 PDT
Received: by pzk29 with SMTP id 29so457028pzk.19
	for <pgsql-hackers@postgresql.org>;
	Fri, 17 Jul 2009 02:54:55 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.142.215.19 with SMTP id n19mr199454wfg.20.1247824495568; Fri,
	17 Jul 2009 02:54:55 -0700 (PDT)
In-Reply-To: <4A4DE104.8090605@pjmodos.net>
References: <4A37BF63.50008@pjmodos.net> <4A37E122.8070303@pjmodos.net>
	<4A38A956.8080600@pjmodos.net> <4A4DE104.8090605@pjmodos.net>
Date: Fri, 17 Jul 2009 15:24:55 +0530
Message-ID: <a301bfd90907170254re7dd52es9fedb007376d8055@mail.gmail.com>
Subject: Re: GRANT ON ALL IN schema
From: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
To: Petr Jelinek <pjmodos@pjmodos.net>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
X-Virus-Scanned: Maia Mailguard 1.0.1
X-Spam-Status: No, hits=0 tagged_above=0 required=5 tests=none
X-Spam-Level: 
X-Archive-Number: 200907/1073
X-Sequence-Number: 141695

Hi,

>
> Attached is v2 with slightly improved code, nothing has changed
> feature-wise.
>

Here are some comments on this patch from my side:

grant.sgml
* Maybe we should use
<replaceable class="parameter">schemaname</replaceable> in the sgml
references instead of just  <replaceable>schemaname</replaceable>

+    There is also the posibility 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 IN schemanema syntax in place of object name.
+   </para>
+
+   <para>

typo "posibility"
It should be ALL [TABLES|VIEWS|SEQUENCES|FUNCTIONS} IN schemaname
(note the other typo here) syntax to be precise IMHO.

aclchk.c
+ 			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
+ 				 (int) objtype);

Kinda funny to mention the C structure name in the error. But I see
that the other functions in the file do the same, so should be ok. I
doubt if the syntax allows any other object type to reach upto this
function anyways :)

parsenodes.h
GrantObjectType objtype;	/* kind of object being operated on */
+ 	bool		is_schema;		/* if true we want all objects
+ 								 * of objtype in schema */

You forgot to make changes in _copyGrantStmt and _equalGrantStmt to
account for this new field.

Rest of the changes look straightforward and ok to me. make
installcheck passes cleanly too. I also do not see any new warnings
due to this patch.

As an aside, I was just wondering the behaviour for RELKIND_INDEX?

Regards,
Nikhils
-- 
http://www.enterprisedb.com