Re: New patch for Column-level privileges

>> BTW, what is the official reviewer's opinion?
>> It seems to me most of the issues on column-level privileges are
>> resolved, so it is almost ready for getting merged.
> 
> I tend to doubt Tom's had a chance to review it again yet, which is
> fine, though I'm certainly hopeful the recent focus and our combined
> efforts mean this patch can be included for 8.4.  My personal opinion is
> that it's ready for beta testing (which is kind of what this feels like
> we're doing here) and barring any serious issues found during testing is
> good to go for inclusion.
> 
> As for areas where there could still be some improvment, I'd love to
> hear your thoughts and opinions (and others!) on how column-level
> privileges are handled in ExecuteGrantStmt and into ExecGrant_Relation
> and how we might improve it.  I don't like the nested for() loops in
> ExecuteGrantStmt, but I don't see any easy way to resolve that and keep
> the SQL-required syntax.

What is your concern?
In my personal opinion, it is quite natural to apply nested-loop to
handle multiple columns within multiple tables.
At least, I don't have a smart idea to handle two-dimensional data
structure withou nested-loop.

> As for ExecGrant_Relation, it'd be nice if we
> could shorten it somehow by either combining the relation and column
> level handling into a single piece of code, or maybe refactoring it into
> seperate functions which could be called from both pieces..

It seems to me ExecGrant_Relation() is a bit larger than other ExecGrant_XXXX()s.
My preference is to clip out column-privilege part into ExecGrant_Attribute()
and invoke it for each given columns.
But, it is just my preference. Please ask it official commiters/reviewers.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>