DASSL / ClassDB

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

Updates to ClassDB Uninstaller and Shelter Schema #87

Closed wildtayne closed 7 years ago

wildtayne commented 7 years ago

This branch contains a few different changes:

Uninstaller:

Shelter Schema:

Misc:

Two further questions:

smurthys commented 7 years ago

Commenting only on shelter schema for now:

  1. I suggest using CREATE SCHEMA IF NOT EXISTS and remove DROP SCHEMA in case the instructor has added to the schema previously. Shelter schema isn't really part of ClassDB, but is provided as a helpful starter. So, it is possible an instructor has extended. (Granted, CREATE TABLE statements will remove any revisions to table schema.)

  2. I don't see a need for ClassDB to own the schema, because it is only an example schema. We do however need all the other privileges as granted to other app roles at the end of the script. At uninstall, we can leave the shelter schema as is, because creating that schema is not part of ClassDB installation.

  3. I love the use of SET ROLE, but it won't be needed if we agree ClassDB won't own the schema.

  4. I propose using SET LOCAL SCHEMA shelter before creating tables and inserting rows and then remove the schema name qualification from object names. This way very few changes will be needed if an instructor wants to add the objects to a different schema, etc.

  5. I propose renaming the files createShelterSchema.sql and dropShelterSchema.sql.

  6. If it is not too much effort, I propose putting the INSERT statements to a file such as populateShelter.sql.

wildtayne commented 7 years ago

Thanks for the feedback. I think all of these changes make sense. Just to confirm how the scripts will behave:

I'll push a new commit in a minute that should address these changes.