introduce extra SQL scripts for OMERO 5.3

[mtbc]

What this PR does

This PR adds a couple of SQL utility scripts for OMERO server admins to use:

• OMERO5.2__0-precheck.sql on an OMERO 5.2 DB checks that OMERO5.3DEV__7/OMERO5.2__0.sql is expected to proceed without errors: there are no ROI keywords or namespaces set and all the transform strings are parseable. It does not itself make any changes to the DB.
• delete-ns-orphans.sql on an OMERO 5.3 DB deletes non-sharable annotations that do not annotate anything and are not set to some blank or OME namespace. This is intended as an optional upgrade step.

Testing this PR

Start out from OMERO 5.2. Test that the precheck script fails if you set,

• a ROI keyword
• a ROI namespace
• a nonsense ROI transform.

(bin/omero obj can be very handy here.)

Set a parseable transform instead (e.g., rotate(30)) and see if the precheck passes.

Annotate some objects with various things: comments, ratings, tags, files, whatever. Unlink some of them without deleting the actual annotation (bin/omero delete's --exclude option might help here). You can put some annotations into your own namespace using bin/omero obj again. (Read ahead a bit to see exactly what to try creating.)

Go ahead and do the DB upgrade to OMERO 5.3 patch 7.

Check that no annotations were deleted.

Now, try running the optional delete script. See that linked annotations, sharable annotations (tags, files, etc.), annotations in your own namespace all survive. Orphaned comments, ratings, etc., that are not in your own namespace should now be deleted.

Related reading

• http://lists.openmicroscopy.org.uk/pipermail/ome-devel/2016-January/003556.html
• outstanding 5.3 DB changes #4608 (comment)
• https://trello.com/c/nLwzmAPc/38-preparatory-sql-scripts

Note that this PR will require a companion documentation PR.

[sbesson]

See https://ci.openmicroscopy.org/view/Failing/job/OMERO-DEV-merge-deploy/347/console, the addition of new forms of SQL scripts break the semantics expected by omego while checking whether upgrades are available.

[mtbc]

I guess it needs a tighter regex, @manics? (Isn't there a card relating to DB version/patch naming? Trello's really hampered by the lack of any decent search.)

[mtbc]

Also happy to just rename the script so that its purpose remains clear but that it doesn't clash with possible version/patch numbers: any suggestions as to what will fit best with omego's expectations?

[sbesson]

At the omego level, the SQL schemas sorting logic is performed DbAdmin.sort_schemas() and might need either to skip SQL files not matching the regexp or to whitelist additional SQL patterns like the one introduced in this script.

[mtbc]

Ideas from chat in slack: perhaps omego should be,

1. tolerant of new scripts that don't fit what it knows how to handle
2. able to handle these precheck scripts.

[manics]

tolerant of new scripts that don't fit what it knows how to handle

Worth discussing- on one hand it makes omego robust against future upgrades, OTOH it's one less check against mistakes in the sql naming.

able to handle these precheck scripts.

Definitely makes sense. If you define the exact naming convention, and how the pre-check should be performed, (e.g. psql -c xxx-precheck.sql should run without error?) this should be easy to implement.

[manics]

Also if you can see a requirement for other scripts which must be run in a certain order (e.g. pre1, pre2, post1, etc) it'd be worth scoping that now.

[mtbc]

Yeah, you just run the precheck script before upgrade, it either errors out or doesn't, and never changes the database. I'd just named it the same as the upgrade script but with a -precheck before the .sql; open to changing that.

I can imagine a postcheck that runs after the upgrade? I don't have a use case though.

I can also imagine optional scripts that one might run, some before the upgrade, some after it, like the other included with this PR. While I'm not suggesting that omego should offer those anytime soon, perhaps you'd like to constrain now how they should be named?

[manics]

At risk of getting too far of topic, how about anything that should be automatically handled follows one naming convention, and everything else follows another (e.g. a different prefix, suffix, or some other unambiguous convention). That deals with my comment checking for mistakes in sql script names.

[mtbc]

@pwalczysko: Probably easiest from psql:

• to set a namespace on Roi 123: UPDATE roi SET namespaces = ARRAY ['penguin'] WHERE id = 123;
• to clear the namespaces on Roi 123: UPDATE roi SET namespaces = NULL WHERE id = 123;
• to set a keyword on Roi 123: UPDATE roi SET keywords = ARRAY ['penguin'] WHERE id = 123;
• to clear the keywords on Roi 123: UPDATE roi SET keywords = NULL WHERE id = 123;
• to set the transform on Roi 123: UPDATE shape SET transform = 'rotate(30)' WHERE roi = 123;
• to clear the transform on Roi 123: UPDATE shape SET transform = NULL WHERE roi = 123;
• to find link for annotation on image 123: SELECT id, child FROM imageannotationlink WHERE parent = 123; (shows link ID then annotation ID)
• to delete annotation link 123: DELETE FROM imageannotationlink WHERE id = 123;
• to see if annotation 123 exists: SELECT COUNT(id) FROM annotation WHERE id = 123; (1 for yes, 0 for no)

Happy to further help / explain.

[mtbc]

Another option would be everything that should not be automatically handled goes into subdirectories.

[pwalczysko]

Start out from OMERO 5.2. Test that the precheck script fails if you set,
a ROI keyword
a ROI namespace
a nonsense ROI transform.
Set a parseable transform instead (e.g., rotate(30)) and see if the precheck passes.

These 4 tests passed.

[joshmoore]

Coming to this late, no objections if we need to add a subdirectory (or even review the general directory layout) but I think that some "well-known" naming scheme (like another __ will/would suffice for detection.

[pwalczysko]

In 5.2 DB, I set up following objects:

• Tags: unique-non-linked, shared-linked, unique-linked
• Comments: linked, non-linked, with-namespace-linked, with-namespace-non-linked
• Ratings: linked, non-linked, with-namespace-non-linked
• MAnnotations: linked, non-linked
• OritinalFile(on the file from import): unique-linked, uniquie-non-linked,
• uploadedFAnnot(special case of OriginalFile in UI, but otherwise the same): unique-linked, unique-non-linked, shared-linked

For these purposes here, the items like "linked", "non-linked" mean "without any manually added namespace and linked", "without any manually added namespace and non-linked" and analogically for all the items except the "with-namespace-xxx" items.
Then I upgraded the DB to OMERO5.3DEV__7/ and checked the items above for presence (no deletions expected). All the items are there after upgrade.

[pwalczysko]

Ran the delete-ns-orphans.sql script on the DB upgraded as described in previous comment.
Then again checked the items described in previous comment.
As expected, only the non-sharable annotations (in this sense this means Ratings, Comments, but not MAnnotations), were deleted which did not have any specific, user-created namespace on them.
Of such, there were only two in my testing set, 1x Rating (non-linked) and 1x Comment (non-linked)

[pwalczysko]

In summary, all the functional tests here passed.

[sbesson]

Moving to breaking to test the deployment with omego 0.4.1

[sbesson]

See https://ci.openmicroscopy.org/view/Failing/job/OMERO-DEV-breaking-deploy/627/, removing breaking marker for inclusion in merge build.

[ghost]

Were we to be using PostgreSQL table inheritance, this could be using DELETE FROM ONLY to avoid deleting from child tables!

[mtbc]

Ha, good point; I wonder how that fits with Hibernate.

[ghost]

Unsure. I imagine that it would be entirely transparent, up until the point you need that bit of nonstandard SQL for the deletes.

[mtbc]

@rleigh-dundee: Good to merge, from your point of view?

[ghost]

Sorry, I didn't comment. I read through and it looked fine to me (though I didn't do any testing). I liked the impressive use of PL/pgSQL for the transform parsing!