Обсуждение: 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