Alvaro,
* Alvaro Herrera (alvherre@commandprompt.com) wrote:
> I had a look at aclchk.c and didn't like your change to
> objectNamesToOids; seems rather baroque. I changed it per the attached
> patch.
I've incorporated this change.
> Moreover I didn't very much like the way aclcheck_error_col is dealing
> with two or one % escapes. I think you should have a separate routine
> for the column case, and prepend a dummy string to no_priv_msg.
I can do this, not really a big deal.
> Why is there a InternalGrantStmt.rel_level? Doesn't it suffice to
> check whether col_privs is NIL?
No, a single statement can include both relation-level and column-level
permission changes. The rel_level flag is there to indicate if there
are any relation-level changes. Nothing else indicates that.
> Is there enough common code in ExecGrant_Relation to justify the way you
> have it? Can the common be refactored in a better way that separates
> the two cases more clearly?
I've looked at this a couple of times and I've not been able to see a
good way to do that. I agree that there's alot of common code and it
seems like there should be a way to factor it out, but there are a
number of differences that make it difficult. If you see something I'm
missing, please let me know.
Thanks,
Stephen
