Thread: Quick coding question with acl fixes
Hi, Usually i'd ply away at this one until I figured it out, but we're running out of time :) If I have the nspForm variable (which is a Form_pg_namespace) struct pointer, then how do I check if the nspacl field is default (ie. "NULL"). This is so I can avoid running DatumGetAclP on it, which seems to cause alloc error reports. gdb seems to think it's "$3 = {126}", and that doesn't seem to be NULL or Datum(0) or anything. Anyone know the answer? Also, there is an implementation question? When I run through the acl changing all references from the old owner to the new owner, should I combine the resulting acls if possible? Because if the new owner already has some acls on that item, they will end up with two acls. Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: > If I have the nspForm variable (which is a Form_pg_namespace) struct > pointer, then how do I check if the nspacl field is default (ie. > "NULL"). This is so I can avoid running DatumGetAclP on it, which seems > to cause alloc error reports. Best practice is to use SysCacheGetAttr if you are dealing with a row from cache or heap_getattr if it's from a table scan. For instance in aclchk.c there is aclDatum = SysCacheGetAttr(NAMESPACENAME, tuple, Anum_pg_namespace_nspacl, &isNull); if (isNull) old_acl = acldefault(ACL_OBJECT_NAMESPACE, ownerId); else /* get a detoasted copy of the ACL */ old_acl = DatumGetAclPCopy(aclDatum); > When I run through the acl changing all references from the old owner to > the new owner, should I combine the resulting acls if possible? Because > if the new owner already has some acls on that item, they will end up > with two acls. If possible ... how painful would it be to do? regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: >> When I run through the acl changing all references from the old owner to >> the new owner, should I combine the resulting acls if possible? Because >> if the new owner already has some acls on that item, they will end up >> with two acls. > If possible ... how painful would it be to do? Actually it looks like you'd better, because for example aclupdate assumes there's only one entry for a given grantor/grantee pair. BTW, are you sure Fabien did not already solve this problem in his pending patch? regards, tom lane
>>If possible ... how painful would it be to do? I'm yet to do that part, so I guess I'll find out. > Actually it looks like you'd better, because for example aclupdate > assumes there's only one entry for a given grantor/grantee pair. OK, many thanks for the prompt reply :) > BTW, are you sure Fabien did not already solve this problem in his > pending patch? You mean schema ownership? I thought that was just upon the first connection to a database or something? I'm using schemas as my first case for fixing OWNER TO commands and acls... Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: >> BTW, are you sure Fabien did not already solve this problem in his >> pending patch? > You mean schema ownership? I thought that was just upon the first > connection to a database or something? Yeah, but the point was that he was doing an ALTER OWNER and needed to fix the ACL to match. I thought he claimed to have written the needed subroutine. I have not yet looked at his patch though. regards, tom lane
> Yeah, but the point was that he was doing an ALTER OWNER and needed to > fix the ACL to match. I thought he claimed to have written the needed > subroutine. I have not yet looked at his patch though. I think Fabien's owner changing routine will end up being a strict subset of my routine. I think his just happens to only work on the newly created public and info_schema in a new db. It's not complex enough to work on arbitrary acls. Also, his needs to work as a public SQL function: + /* acl acl_switch_grantor(acl, oldgrantor, newgrantor); + * switch grantor id in aclitem array. + * used internally for fixing owner rights in new databases. + * must be STRICT. + */ + Datum acl_switch_grantor(PG_FUNCTION_ARGS) + { + Acl * acls = PG_GETARG_ACL_P_COPY(0); + int i, + old_grantor = PG_GETARG_INT32(1), + new_grantor = PG_GETARG_INT32(2); + AclItem * item; + + for (i=0, item=ACL_DAT(acls); i<ACL_NUM(acls); i++, item++) + if (item->ai_grantor == old_grantor) + item->ai_grantor = new_grantor; + + PG_RETURN_ACL_P(acls); + } Chris
>>When I run through the acl changing all references from the old owner to >>the new owner, should I combine the resulting acls if possible? Because >>if the new owner already has some acls on that item, they will end up >>with two acls. > > If possible ... how painful would it be to do? I'm pretty close to finishing this. Just applying it to all the acl'able objects atm. It's a pain when you can only find 15 mins every other day to work on it :( Chris