Re: WIP: Column-level Privileges

Alvaro,

* Alvaro Herrera (alvherre(at)commandprompt(dot)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