Hi,
Markus Wanner wrote:
>> As mentioned in above, regression tests, documentation updates,
>> dependency handling, and actually implementing the permission checks all
>> remain. What I'm looking for feedback on are the changes to the
>> grammer, parser, catalog changes, psql output, etc.
>
> Aha, good. So I'm going to (try to) check these things and comment.
Sorry, this took way longer than planned.
The grammar and parser changes look fine to me. You've added a 'Priv'
parser node which now stores a privilege string (like 'REFERENCES',
'SELECT' or 'CREATE') as well as a list of affected columns.
I've been wondering about the use of 'ColId' instead of all the other
options (i.e. 'UPDATE', 'DELETE', 'TRUNCATE', ...). But that has
obviously been there before. Checking it is deferred to later giving an
"unrecognized privilege type" error. I'm wondering why this is done that
way. Seems to be related to some unreserved_keywords vs col_name_keyword
vs reserved_keywords issue.
However, the following is certainly bogus and needs to be prevented by
the parser or later privilege type checking code:
testdb=# GRANT TRUNCATE (single_col) ON test TO malory;
GRANT
Otherwise the syntax seems to match what my SQL 2008 draft is telling.
MySQL does it the same way as well.
The catalog changes have been discussed with Tom.
Some privilege regression tests currently fail with your patch, but I
think that's expected.
Documentation and new regression tests for column level privileges are
still missing. If you want, Stephen, I can work on that.
Given the known-unfinished state of this patch I'm moving it to the
November commit fest. Hope that's fine with you. I'm glad to help and
review as updated patch, no matter what the commit fest state is.
Regards
Markus Wanner