Forum OpenACS Development: Cannot delete groups with segments and rels

After adding relational segments to a group and then relating parties to the group through those segments, it is impossible to remove the group as designed through pl/pgsql function acs_group__delete. This function is called in the normal dismantling of an application_group with tcl proc application_group_delete.

The reason for the failure is that acs_group__delete attempts to remove the segments before the rels are removed, rusulting in:

ERROR:  party_member_party_fk referential integrity violation - key in parties still referenced from party_approved_member_map

This problem and it's fix at https://openacs.org/bugtracker/openacs/bug?bug_number=775 applies to 4.6.x and 5.0.

Randy

Collapse
Posted by tammy m on
Hi

Well the good news is I applied your patch and deleting groups now works except when I've already assigned permissions to the group or its rel_segs. Then I get ERROR: acs_permissions_grantee_id_fk referential integrity violation - key in parties still referenced from acs_permissions.

Maybe acs_group__delete could add code to drop permissions first? Or a delete cascade on group and rel_segs? I haven't found a way to mass delete permissions yet, only permission::revoke.

Am I missing something more obvious here?

Collapse
Posted by Randy O'Meara on
Tammy, where in the sequence are you seeing the permissions RI error? What does your code look like? What call are you using the zap the group?

I've seen a few other forums discussions regarding cascading deletes. There are some that think the data model is seriously flawed because it doesn't do enough of that. I wouldn't expect a suggestion in that direction to be accepted without a great deal of...

Collapse
Posted by tammy m on
Hi again Randy,

Well I wouldn't want "a great deal of... !" I got a bit of that already in a posting on named parameters. Not going there again if I can help it;)

I'm just calling the acs_group__delete pl/sql function that you posted here directly in psql. I'm trying to delete an app group that I created with

 # Create an application group (group associated with a package instance).
  set app_grp [application_group::new -group_type application_group \
                  -package_id $package_id  \
                  -group_name "$pkg_name AppGroup" ]

Then I added subgroup and permissions like so

        set pm_seg [rel_segments_new $app_grp membership_rel "$pkg_name Members"]
        permission::grant -party_id $pm_seg \
                -object_id $package_id -privilege create

I put some debugs in acs_group__delete and it is getting the constraint violation on the group delete itself.

NOTICE:  BEGIN Deleting relation composition_rel, id 7420
NOTICE:  END Deleting relation composition_rel(7420)
NOTICE:  BEGIN Deleting segment 7422
NOTICE:  END Deleting segment 7422
NOTICE:  BEGIN Deleting segment 7425
NOTICE:  END Deleting segment 7425
NOTICE:  BEGIN Deleting group 7417
NOTICE:  END Deleting group 7417
ERROR:  acs_permissions_grantee_id_fk referential integrity violation - key in parties still referenced from acs_permissions

I'll look more into this after lunch... are you able to use acs_group__delete (your modified form) successfully even with permissions granted on your group? I granted permissions directly to my group... thinking that permissions the group has directly would be inherited by all subgroups (relational segments of the group). Is that true?! And maybe that's my problem, I need to drop the permissions directly on my group as well as on the rel_segs.

  # Grant privileges: application group.
  # Grants privileges to all rel_segs of application group.
  permission::grant -party_id $app_grp \
                -object_id $package_id -privilege read

Duh. More on this after lunch...

thanks again

Collapse
Posted by Randy O'Meara on
Hi Tammy,

Uh, yeah. I saw the exchange you mention...

I have not been dealing with app groups directly unless I have to. I use the subsite as a container, along with its application group that gets created when a subsite is instantiated. I use the apm to do the instantiation. I then supplement the subsite app group with additional segments to create more roles, assign rights to the segments, and then add rels in a particular role. See the "howto" thread (https://openacs.org/forums/message-view?message_id=116231) for details.

When I located the bug mentioned in this thread, I was using the apm to destroy a subsite after destruction of all child site nodes. If you really want to do the app group stuff yourself, do it with the apm first and look at the log to see how/when the permissions are deallocated. I use the toolkit's magic whenever I can.

Randy

Collapse
Posted by tammy m on
Ok

After more testing in psql, I was wrong, I am not able to delete a rel_seg with permissions previously granted on it or an app group with existing permissions granted on it. I get the same constraint violation for both attempts.

Collapse
Posted by tammy m on
Bah!

More bad news for me. When I attempt to delete the group from the Groups Admin page (Nuke this group), I get the same foreign key violation on my group.

[04/Sep/2003:15:32:53][5032.19476][-conn:openacs-dev::16] Notice: Querying 'select __exec_43_delete_group ();'
NOTICE:  BEGIN Deleting relation composition_rel, id 7420
NOTICE:  identifier "rel_constraint__violation_if_removed" will be truncated to "rel_constraint__violation_if_re"
NOTICE:  END Deleting relation composition_rel(7420)
NOTICE:  BEGIN Deleting segment 7422
NOTICE:  END Deleting segment 7422
NOTICE:  BEGIN Deleting segment 7425
NOTICE:  END Deleting segment 7425
NOTICE:  BEGIN Deleting group 7417
NOTICE:  END Deleting group 7417
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Error: Ns_PgExec: result status: 7 message: ERROR:  acs_permissions_grantee_id_fk referential integrity violation - key in parties still referenced from acs_permissions

[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Error: dbinit: error(localhost::openacs-dev,ERROR:  acs_permissions_grantee_id_fk referential integrity violation - key in parties still referenced from acs_permissions
): 'select __exec_43_delete_group ()'
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Notice: Querying 'drop function __exec_43_delete_group ();'
NOTICE:  current transaction is aborted, queries ignored until end of transaction block
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Notice: dbinit: sql(localhost::openacs-dev): 'drop function __exec_43_delete_group ()'
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Notice: Querying 'abort transaction;'
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Notice: Ns_PgExec: Rolling back transaction
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Notice: dbinit: sql(localhost::openacs-dev): 'abort transaction'
[04/Sep/2003:15:32:55][5032.19476][-conn:openacs-dev::16] Notice: RP (3266.875 ms): error in rp_handler: serving GET /admin/groups/delete-2?group_id=7417&operation=Yes%2C+I+really+want+to+delete+this+group 
        ad_url "/admin/groups/delete-2" maps to file "/www/nsroot/openacs-dev/packages/acs-subsite/www/admin/groups/delete-2.tcl"
errmsg is Transaction aborted: Database operation "0or1row" failed (exception NSDB, "Query was not a statement returning rows.")

Makes sense since ultimately it ends up calling acs_group__delete just like I tried in psql!

So if I do this first

delete from acs_permissions where grantee_id in (select segment_id from rel_segments where group_id = 7417);
delete from acs_permissions where grantee_id = 7417;

then my call to acs_group__delete(7417); works fine. I think something to this effect should be added to acs_group__delete pl/sql function so that the group delete just works.

Collapse
Posted by Randy O'Meara on
Tammy,

There is a bug (https://openacs.org/bugtracker/openacs/bug?bug_number=234), filed by Tilmann back in February, that discusses the bothersome necessity to delete permissions assigned to an object prior to deleting the object. The problem is much larger and more pervasive than the groups issue you encountered. The ensuing discussion seemed to establish that a cascaded delete from acs_objects to acs_permissions was appropriate.

The patch (https://openacs.org/bugtracker/openacs/patch?patch_number=74) that was submitted to close the bug on that same day doesn't address a cascaded delete that I can see. It does "delete from acs_permissions..." when an object is deleted. Which should guarantee that permissions are deleted before the acs_object.

Somebody that's more familiar with the groups system needs to state whether the suggestion you make (deleting perms from within acs_group__delete) is essentially equivalent to the issue that was resolved by Til's bug/patch, and then apply that change if it's appropriate.

My vote would support such a change if some kernel guru thinks it's the right thing to do.

Don? Til? Jeff?

Collapse
Posted by Andrew Piskorski on
I don't think I understand where the "cascaded deletes" in question are coming from here, I don't see any mentioned of them above. But when it comes to bug 234, of course a cascaded delete there won't do the right thing, it can't. AFAIK the only proper way there has ever been to delete an acs_object is by calling acs_object.delete() on the row. Properly, any core helper objects, like permissions, should also be deleted when acs_object.delete() is called, which is what the patch for that bug fixed, basically by just doing "delete from acs_permissions where object_id = v_object_id".

If you want "on delete cascade" to Do the Right Thing in general with acs_objects (perhaps a worthwhile goal...), that's a different and larger issue. AFAIK it is currently not supported by the OpenACS object system.

On the the deleting groups problem above, yup, sounds to me like a similar patch needs to be made there. It's the other end of the same problem actually, when you delete an acs_object you need to delete the permissions granted on that object. Likewise when you delete a group which has permissions on some object, you need to delete those granted permissions before deleting the group too...

Collapse
Posted by tammy m on
Hi,

I took a look around to see if maybe there is anything else besides permissions, relations and rel_segs that would make sense to be deleted automatically when deleting a group. I figure since Randy submitted a bug and patch for deleting groups, we might as well get as much of this handled now with that bug as possible!

I think there might be a few more items... here are the steps I think need to be done to delete a group.

  1. delete rel_constraints.required_rel_segment on this group
  2. delete permissions on group and all its rel_segs
  3. delete dependent constraints on group and it's rel_segs (like rel_segments_delete tcl proc)
  4. delete relations to group and rel_segs

The 1st step is based on code I found from where a group is deleted from the admin page (packages/acs-subsite/www/admin/groups/delete-2.tcl); group::delete does an extra step before it calls application_group__delete:

  -- the acs_group package takes care of segments referred
  	    -- to by rel_constraints.rel_segment. We delete the ones
	    -- references by rel_constraints.required_rel_segment here.
for row in (select cons.constraint_id
                          from rel_constraints cons, rel_segments segs
                         where segs.segment_id = cons.required_rel_segment
                           and segs.group_id = :group_id) loop

                rel_segment.delete(row.constraint_id);

            end loop;
Seems the tcl proc group::delete is the right/preferred way to delete app groups. It looks like an extra rel_constraint may be left lying around if I call pl/sql function acs_group__delete directly instead of tcl proc group::delete. Maybe this constraint is created for me when I create my app group (on it's relationship to the app group rel_type)? I think any constraints I might create on my rel_segments are deleted differently (like in rel_segments_delete)? I'm not sure about the constraint deletion here...

The 2nd I had to do or my call to acs_group__delete would fail with ERROR: acs_permissions_grantee_id_fk referential integrity violation - key in parties still referenced from acs_permissions

I know I didn't create any rel_constraints on my group so I don't care about the 3rd... but might be a "bug" for someone else and cause acs_group__delete to fail;(

Anyway, there is definitely more to deleting a group than what the current acs_group__delete does and it would be really helpful to get most of this done in Randy's patch to the proc so that it gets into the code base soon:) Can a core team developer let us know what the right things to do in this proc are?

For instance rel_types to the group or it's rel_segs would also need to be removed if they were created. But I don't think that's a good thing to do in acs_group__delete itself since rel_types are reusable among groups and may not have been created strictly for use with a single group. I think deleting rel_types should be the creator's decision to explicitly delete.

And, just for the record, I am still able to drop my groups after I have added members to them too. I tried that today;)

Guidance from those who know on the final patch to acs_group__delete?! thanks:)

Collapse
Posted by Randy O'Meara on
Tammy,

That's really some great and thorough work. I can only hope that this thread has gained the attention of the core folks. I know that several people (including core folk) have complained in these forums about having to address deleting permissions explicitly. I think it's great that you took the time to thoroughly analyze *all* of the shortcomings of group deletion.

Many of the people that would know if your fix is correct are very busy at the moment preparing the 5.0 code for release. It would be a shame for this to fall through a crack because it's not addressed at this point in time.

Collapse
Posted by tammy m on
Randy

Good point... everyone is busy and this could get lost... I added a comment with a link back here to your submitted bug patch. Thanks for creating the patch:)

Collapse
Posted by Randy O'Meara on
Tammy,

Maybe a better way to gurantee acceptance with minimum fuss is to replace my patch with yours, which should contain my changes and yours. I believe you can post another patch to the same bug.

Better yet would be to do the above *and* provide an aa_test case (https://openacs.org/forums/message-view?message_id=121426) to show failure/success?

Randy

Collapse
Posted by tammy m on
Yer right, of course. Posted a patch. I just added code to drop permissions on the group and its rel_segs to your original patch, Randy.

I still think that 2 more additions might need to be made but doubt I am the person to decide +/or do that! Just not intimate enough with the groups system and the OACS design philosophy/decisions on what should be done in pl/sql functions versus tcl procs that call those pl/sql functions. I would opt for more in pl/sql; tcl just calls the db function for ya. That way developers would not have to worry if they "got it all" when they used db functions versus seemingly corresponding tcl procs.

Anyway, here are the 2 other steps I think need to be done to fully delete a group.

In packages/acs-subsite/tcl/group-procs.tcl, group::delete does the extra step (mentioned above in this same thread) of removing rel_constraints.required_rel_segment before calling acs_group__delete (via application_group__delete). I would think the code to delete the rel_constraints.required_rel_segment should be moved to acs_group__delete so that groups could be deleted in one shot via the db function or the tcl proc. But the decision to keep this code in the tcl proc may have been made for design/architecture reasons I'm not grokking yet.

In packages/acs-subsite/tcl/rel-segments-procs.tcl, rel_segments_delete segment_id (which are identified by the rel_constraints.required_rel_segment field as well), does an extra step of deleting dependent constraints before calling rel_segment__delete db function. As above, the code to delete the dependent constraints is not executed if you call the plsql function instead of the tcl proc.

Collapse
Posted by Andrew Piskorski on
Tammy, I'd say your intuition is right: All functionality needed to properly delete an object should be in the PL/SQL delete function for that object, not spread half and half between the PL/SQL and Tcl code. Some further evidence for this is that the group::delete Tcl proc appears to have an old comment dating from the original 2000 implementation, "Maybe the relational constraint deletion should be moved to the acs_group package..."

So I'd bet good money that the current situation with group::delete is a historical, path of least resistance accident ("well, it's ugly but this way I don't have to write a database upgrade script"), not the result of any careful design.