classdb.listOrphans() and uninstaller orphan object check

[wildtayne]

This branch implements the fix outlined in #107 for orphan objects:

• classdb.listOrphans() was added to addHelpers.sql

• This function lists objects which are owned by classdb_instructor and classdb_dbmanager. It returns a table containing: name of the owner, name of the object, schema of the object, and the object type.

• Currently, this function only looks for objects which are in pg_class and pg_proc. This includes: tables, views, indexes, sequences, mat. views, types, TOAST tables(?), foreign tables, and functions.

• These are used over info_schema views because each of these tables contains the owner of the object.

• dropInstructor and dropDBManager now perform REASSIGN OWNED BY TO after their schemas are dropped

• removeFromDB checks if classdb.listOrphans() returns any rows. If so, it throws an exception instructing the user to execute classdb.listOrphans() and remove or reassign those objects.

• There are probably some object types not covered by this, but the major ones are. I intentionally left out triggers, since triggers have to be associated with a table, so the user will find out about the trigger when dropping the table. (and instructors/dbmanagers can't create EVENT TRIGGERS)

(Fixes #107)

[afig]

Changes look good, This page seems to list the available database objects (although it does not promise to be comprehensive).

Could you also include a fix to the Issue that I recently opened, #117, since the same file is being modified?A potential fix is outlined in a Gist linked to in a comment. The exact function cannot be copy and pasted since other changed were introduced in this branch. Thanks

[smurthys]

In listOrphans, the last column's name is misspelled in the table type returned: has an extra 'u'

In dropInstructor and dropDBManager, it will be helpful if a notice is raised on reassignment (requires a test for existence of objects that need to be reassigned).

In removeFromDB.sql

• Check for orphans first: let the script fail fast, immediately after the test for superuser
• There is no guarantee classdb.listOrphans is created. Check if that function exists before using it. Just move on if the function does not exist. (There are other implications if listOrphans does not exist, but we can discuss that after M1.)
• Unrelated to this commit: I wonder if the REVOKE and DROP OWNED BY statements cause notices that could be suppressed. Move up the SET LOCAL client_min_messages... (to after the block involving listOrphans)?

[wildtayne]

Andrew - no problem, I can also add that fix

[wildtayne]

Thanks for the review, I just pushed a commit to address these issues.

• Moved ALTER DEFAULT PRIVILEGES statement in the dropInstructor function (Fixes #117)
• Fix misspelling in listOrphans
• Created classdb.listOwnedObjects() function, which lists all objects owned by a single role (basically uses the query from classdb.listOrphans()
• Changed listOrphans() to call listOwnedObjects() for instructor and dbmanager
• Used listOwnedObjects to check if any objects are owned by dbmanagers and instructors on drop. A notice is raised if there are any.
• Moved orphan check to top of removeFromDB
• Check if listOrphans and listOwnedObjects exist in removeFromDB
• Moved the notice suppression in removeFromDB to cover all statements where we do not explicitly throw a notice.

[smurthys]

listOwnedObjects. 👍

• Parameter userName could be renamed to roleName because it can be any role; not just a login role
• I wonder if column owner is redundant since it will be the same value always
• The other columns could be named object, schema, and kind

Rename listOrphans() to listOrphanObjects(): retain the owner column

• Consider adding an optional parameter which if it begins with i (any case), list orphans from instructor; if it begins with d, list orphans from dbman; else return the union
• If you add the optional parameter as above, revise the notice in dropInstructor and dropDBManager to the exact call users should make, e.g., classdb.listOrphans('instructor') (too bad we can't let the user specify the actual role name that caused the orphans)

removeFromDB.sql: Cannot assume classdb.isSuperuser is created. Please revert to the earlier code.

[wildtayne]

• I've made the suggested changes to listOrphanObjects() and listOwnedObjects
• I also changed the notice given when dropping an instructor/dbmanager to match the new function
• I switched the parameter order for describe(). I moved back to using an override to do this. (Fixes #118)
• I also made describe() and listTables() more closely match the output of \d and \dt respectively.
• I reverted the superuser code in removeFromDB

[afig]

Changes and behavior look good.

Oddly enough, other objects (that I did not create) ended up as orphan objects. Namely "toast" objects. This may have been due to the use of the VARCHAR datatype without a number (equivalent to TEXT).

sql0708=# SELECT classdb.createInstructor('testinss', 'noname');
 createinstructor
------------------

(1 row)

sql0708=# \q
---------------------------------------------------------------------------Switch to testinss
sql0708=> CREATE TABLE public.testtab(col1 VARCHAR);
CREATE TABLE
sql0708=> \q
---------------------------------------------------------------------------Switch back to superuser
sql0708=# SELECT classdb.dropInstructor('testinss');
NOTICE:  Objects belonging to this user exist outside their schema. They have been reassigned to ClassDB_Instructor, and can be viewed by executing classdb.listOrphanObjects('Instructor').
 dropinstructor
----------------

(1 row)

sql0708=# SELECT classdb.listOrphanObjects('Instructor');
                    listorphanobjects
----------------------------------------------------------
 (ClassDB_Instructor,testtab,public,Table)
 (ClassDB_Instructor,pg_toast_22576,pg_toast,TOAST)
 (ClassDB_Instructor,pg_toast_22576_index,pg_toast,Index)
(3 rows)

Information on TOAST:

To overcome this limitation, large field values are compressed and/or broken up into multiple physical rows. This happens transparently to the user, with only small impact on most of the backend code. The technique is affectionately known as TOAST (or "the best thing since sliced bread"). The TOAST infrastructure is also used to improve handling of large data values in-memory. ... If any of the columns of a table are TOAST-able, the table will have an associated TOAST table, whose OID is stored in the table's pg_class.reltoastrelid entry. On-disk TOASTed values are kept in the TOAST table, as described in more detail below.

We may choose to ignore this quirk, not include toast in the values listed by listOrphanObjects, or something else.

An while we're on the topic of silly names, loo as a shorthand for listOwnedObjects might not be the best choice, but I suppose it'll do for now.

[wildtayne]

I think it would be good to display all the objects returned in case some of them were manually created, although I guess that kind of contradicts my reason for excluding triggers. It looks like TOAST is pretty much always an automatic thing, so it would probably be OK to exclude it in a future version. We might want to think about which object types can be excluded for the same/similar reason.

[smurthys]

I see @afig and I had an observation in common. :)

The TOAST business is interesting. I have a feeling it might make a return as an issue. :(

Good work with this commit @srrollo.