DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
7 stars 2 forks source link

Updated uninstall script for M2 #161

Closed wildtayne closed 6 years ago

wildtayne commented 6 years ago

This PR updates the uninstall script to work with M2. I have changed to using the DROP OWNED BY ClassDB approach outlined here. We can discuss if this solution is sufficient in our next sync, if necessary.

This method should no longer forcefully drop user objects that depend on ClassDB objects.

Fixes #147 and #146

wildtayne commented 6 years ago

Closing until dir. reorg is done

wildtayne commented 6 years ago

Reopened and editing PR comment

wildtayne commented 6 years ago

I just pushed another commit that sets ClassDB.IDNameDomain's owner to ClassDB, while before it was owned by the superuser. This is necessary for DROP OWNED BY ClassDB to work, since many of the ClassDB functions depend on ClassDB.IDNameDomain.

When testing the uninstaller, I would recommend creating a new install of ClassDB from the scripts in this branch, as many missing assigns and grants have been added in dev. I ran into several problems running the uninstaller that were due to old functions that have since been removed from ClassDB, or incorrect function permissions that are now fixed.

smurthys commented 6 years ago

Nice changes @srrollo. Using DROP OWNED BY is a good idea. (I should have suggested this approach earlier, because function dropRole already uses DROP OWNED BY.)

I have the following thoughts on removeAllFromDB.sql:

Also, thank you for adding ownership to ClassDB.IDNameDomain. I'd missed doing that.

smurthys commented 6 years ago

FY, I'll be away for 60-90 minutes. So, please feel free to merge this PR unless one of you discovers something major. I'll approve the PR in anticipation of there being no such problem.

afig commented 6 years ago

Thanks for working on the uninstall script @srrollo. I tried running it on a clean M2 install, but received the following error (showing full output since it may be helpful in determining where the script fails):

classdbtest=# \i removeAllFromDB.sql
START TRANSACTION
SET
DO
DO
DO
DROP OWNED
DROP OWNED
DROP OWNED
DROP EVENT TRIGGER
DROP EVENT TRIGGER
DROP FUNCTION
DROP FUNCTION
DROP FUNCTION
DROP FUNCTION
psql:removeAllFRomDB.sql:98: ERROR:  cannot drop desired object(s) because other objects depend on them
DETAIL:  event trigger triggerddlcommandsqldrop depends on function classdb.logddlactivity()

Should DROP EVENT TRIGGER IF EXISTS triggerDDLCommandSqlDrop; be added to the uninstall? Likewise, should the accompanying triggerDDLCommandEnd also be added? These two triggers are defined in addDDLActivityLoggingReco.sql

Also a couple of minor comment observations:

wildtayne commented 6 years ago

Thanks for pointing this out @afig. I had somehow pasted the wrong names for the event triggers in the DROP EVENT TRIGGER statements. The latest commit should fix this.

triggerDDLCommandSqlDrop and triggerDDLCommandEnd are the correct triggers to drop