Forum OpenACS Development: Create and drop scripts and acs_privileges

Are the drop scripts supposed to remove the privileges created by the create scripts? I did the following to test the create and drop scripts.

psql -f survsimp-create.sql openacs4
psql -f survsimp-drop.sql openacs4
psql -f survsimp-create.sql openacs4

When I do that last command (reloading the data models), I get an error which states that I can't create a privilege with the same key. Should I use the acs_privilege__drop_privilege and acs_privilege__remove_child commands in the drop scripts?

Thanks

If the general-comments module is any indication, then, yes the drop script should also drop the priviliges. The drop script must undo all actions in the create scripts.

More in general, AFAIK, the drop script should remove *every* trace of the module from the database. For example, if your module uses general-comments you should remove all comments from the db related to your tables (bad example, maybe, 'on delete cascade' should take care of that).

Collapse
Posted by Don Baccus on
Pascal's right - all traces should be removed.  Most packages don't use "on delete cascade" so you need to chug around looking for things to delete.  Things get missed which is why you see things like dangling permissions.

Judicious use of "on delete cascade/set null" would help with this problem, a lot.  I couldn't help but notice that Ian used "on delete" freely in the portals package.

Maybe that's why he got laid off :)

Uhm. No. I think you really do need to chug through all entries. 'on delete cascade' would only delete the records for which the record with primary-key that is referenced gets deleted. I could drop the general_comments table, and leave all the acs_messaging & acs_object records intact. I would consider those dangling objects. It might be wise to create 'on delete' triggers for all tables... We are not using the object-oriented features of the DB, so it doesn't know about the relations.

Anyway, this is whats in general-comments-drop.sql (for postgres):

create function inline_0 ()
returns integer as '
declare 
    comment_rec RECORD; 
begin

    FOR comment_rec IN select comment_id from general_comments LOOP

        -- There is a bug in content_item.delete that results in
        -- referential integrity violations when deleting a content
        -- item that has an image attachment. This is a temporary fix
        -- until ACS 4.1 is released.
        delete from images
        where image_id in (select latest_revision
                            from cr_items
                            where parent_id = comment_rec.comment_id);

        perform acs_message__delete(comment_rec.comment_id);

    END LOOP;

    return 0;
end;' language 'plpgsql';

select inline_0 ();
Does anybody (Dan?) know whether or not I still need to do the explicit deletion of images?
Collapse
Posted by Don Baccus on
No, "on delete cascade" doesn't solve all of the problems, but it solves one very difficult problem.  I assume it's difficult because it's an area where aD hasn't done a good job solving it in the past:

You build package A.  Later on, someone builds package B which references rows in tables created by A.

The author of package A has no way to drop the bits of package B which  refer to package A's tables.  Using "on delete cascade/set null" in package B solves this problem.  The author of package B could also use  triggers to do this (which, after all, is how "on delete" is implemented) or modify package A (which hard-codes a dependency on B in A, which sucks big time).  This last approach was the traditional ACS approach in 3.x and folks and people forever forgot to make the necessary changes to package A.  This is why the "nuke user" script has hardly ever worked.

In other words, the author of package A has to take responsibility for  cleaning up any garbage it introduces into the system, most certainly.  But package B also needs to clean itself up if package A goes away.  "on delete" is a easy and clean way to do so, one that's not nearly as prone to failure due to forgetfulness as was done in 3.x.

When Philip et al first began working with Oracle, I don't think "on delete" was implemented, it's fairly recent AFAIK.

Collapse
Posted by Dan Wickstrom on
Pascal - looking at the code, it appears that the explicit deletion of images should be unnecessary.  content_item__delete calls content_revision_delete which in turn will call acs_object__delete to delete the content revision and any sub-types that depend on the 'content revision' type.  Since images are a subtype of 'content_revision', everything should be happy.  I would suggest just trying it out, but Vinod has recently uncovered a pg bug that affects content_revision__delete which in turn affects content_item__delete, so until we get that fix, you can't really test it.

Vinod - have you heard anything back about your bug submission?

Collapse
Posted by Vinod Kurup on
Vinod - have you heard anything back about your bug submission?

I posted the bug to pg-hackers and Stephan Szabo commented on it but I haven't heard anything more about it there. I sent a message to Tom Lane and he mentioned that foreign keys were Stephan Szabo's and Jan Wieck's turf. I'll send a message to Stephan Szabo and see what the status is.

Collapse
Posted by Gilbert Wong on
Ok.  So what I get from this discussion is that I should remove all traces of this package.  Since I will be using the content repository for blobs, I should also nuke all of those references, correct?

I think I remember seeing the "on delete cascade" in the simple survey package so I should add those back in.  Where can I find the documentation for "on delete cascade/set null".

Thanks.

Collapse
Posted by Don Baccus on
(ahem) try the "create table" entry in the PostgreSQL reference guide for PG 7.1.  It explains the foreign key stuff, including actions, succinctly but accurately.