outstanding 5.3 DB changes

[mtbc]

• Do not leave "Foo" in pixels table.
• Clean up lingering comments, ratings, etc., for images that no longer exist.

Neither old nor new images should have a Pixels.sha1 of "Foo". For instance,

bin/omero hql "SELECT id FROM Pixels WHERE sha1 = 'Foo'"

should return no IDs.

Also, there should no longer be any comments, ratings, etc. that are orphaned. To create synthetic ones before upgrade just delete their links directly using psql or via CLI, e.g.,

bin/omero delete Image/ImageAnnotationLink:1234 --exclude Annotation --report

Of course, orphaned tags, attachments, etc., ought to remain.

[ghost]

I do really like the Foo-related changes here!

[sbesson]

Tested the upgrade script against the latest version of the valuable DB which had the following content pre-ugrade:

originalvaluable=> SELECT distinct(discriminator) FROM annotation WHERE
originalvaluable->         id NOT IN (SELECT parent FROM annotationannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM annotationannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM channelannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM datasetannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM detectorannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM dichroicannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM experimenterannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM experimentergroupannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM filesetannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM filterannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM imageannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM instrumentannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM lightpathannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM lightsourceannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM namespaceannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM nodeannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM objectiveannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM originalfileannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM planeinfoannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM plateacquisitionannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM plateannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM projectannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM reagentannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM roiannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM screenannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM sessionannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM shapeannotationlink)
originalvaluable->         AND id NOT IN (SELECT child FROM wellannotationlink);
    discriminator     
----------------------
 /basic/num/double/
 /type/OriginalFile/
 /basic/term/
 /basic/num/long/
 /basic/time/
 /basic/text/comment/
 /basic/text/tag/
(7 rows)

originalvaluable=> SELECT count(*) FROM Pixels WHERE sha1 = 'Foo';
 count 
-------
  2102
(1 row)

After running the upgrade scripts, everything is cleaned up as expected:

valuable=>     SELECT distinct(discriminator) FROM annotation WHERE
valuable->         id NOT IN (SELECT parent FROM annotationannotationlink)
valuable->         AND id NOT IN (SELECT child FROM annotationannotationlink)
valuable->         AND id NOT IN (SELECT child FROM channelannotationlink)
valuable->         AND id NOT IN (SELECT child FROM datasetannotationlink)
valuable->         AND id NOT IN (SELECT child FROM detectorannotationlink)
valuable->         AND id NOT IN (SELECT child FROM dichroicannotationlink)
valuable->         AND id NOT IN (SELECT child FROM experimenterannotationlink)
valuable->         AND id NOT IN (SELECT child FROM experimentergroupannotationlink)
valuable->         AND id NOT IN (SELECT child FROM filesetannotationlink)
valuable->         AND id NOT IN (SELECT child FROM filterannotationlink)
valuable->         AND id NOT IN (SELECT child FROM imageannotationlink)
valuable->         AND id NOT IN (SELECT child FROM instrumentannotationlink)
valuable->         AND id NOT IN (SELECT child FROM lightpathannotationlink)
valuable->         AND id NOT IN (SELECT child FROM lightsourceannotationlink)
valuable->         AND id NOT IN (SELECT child FROM namespaceannotationlink)
valuable->         AND id NOT IN (SELECT child FROM nodeannotationlink)
valuable->         AND id NOT IN (SELECT child FROM objectiveannotationlink)
valuable->         AND id NOT IN (SELECT child FROM originalfileannotationlink)
valuable->         AND id NOT IN (SELECT child FROM planeinfoannotationlink)
valuable->         AND id NOT IN (SELECT child FROM plateacquisitionannotationlink)
valuable->         AND id NOT IN (SELECT child FROM plateannotationlink)
valuable->         AND id NOT IN (SELECT child FROM projectannotationlink)
valuable->         AND id NOT IN (SELECT child FROM reagentannotationlink)
valuable->         AND id NOT IN (SELECT child FROM roiannotationlink)
valuable->         AND id NOT IN (SELECT child FROM screenannotationlink)
valuable->         AND id NOT IN (SELECT child FROM sessionannotationlink)
valuable->         AND id NOT IN (SELECT child FROM shapeannotationlink)
valuable->         AND id NOT IN (SELECT child FROM wellannotationlink);
    discriminator    
---------------------
 /type/OriginalFile/
 /basic/text/tag/
(2 rows)

valuable=> SELECT count(*) FROM Pixels WHERE sha1 = 'Foo';
 count 
-------
     0
(1 row)

valuable=> SELECT count(*) FROM Pixels WHERE sha1 = 'Pending...';
 count 
-------
  2121
(1 row)

Note #4585 should ensure the deletion of these orphaned annotations in new servers.

Moving away from "Foo" is certainly a good idea. Could "Pending..." be misleading in the sense it gives the impression that the SHA1 is getting calculated? That being said, I do not have any better suggestion other than "N/A" if we don't want to go for "".

[mtbc]

"Pending..."'s otherwise used by PixelsImpl anyway and we may yet decide to calculate them. 😃

[sbesson]

Fair enough. I have no strong opinion on the Pixels.sha1 terminology. I will run an additional query against a production database to assess the impact of the proposed deletion /cc @kennethgillen and leave time for others to comment/veto the proposed changes /cc @joshmoore. Also looking at the [http://trac.openmicroscopy.org/ome/ticket/2999](original ticket), would there some follow-up work on the OMERO.web client?

[mtbc]

OMERO.web changes already done in eab13c5.

[sbesson]

FOr information, the query on the nigthshade DB leads to ~30K annotations that would be removed by this upgrade step. @kennethgillen / @joshmoore: is there any additional information we should fetch that would help us stay on the safe side before merging this PR?

[mtbc]

Note also that my testing reveals that when deleting a rating in Insight, that rating is actually left behind in the DB. (This is fixed by #4585.) I'd not be shocked if there are other code paths that act similarly.

[chris-allan]

We use orphaned CommentAnnotations and TermAnnotations with a known namespace to store a variety of configuration or user interface related logic on a per user basis. Blindly removing them is going to be problematic.

[mtbc]

Could certainly limit deletion by namespace so any unexpected are left intact.

[mtbc]

The regions branch is retiring and it's not worth reviewing this PR any further until production DBs are checked and limiting deletion by namespace is approved.

[sbesson]

👍 Is there a placeholder for the production DBs investigation including the query script and a summary of the results?

[mtbc]

I know of only https://trello.com/c/QrfLtInf/68-should-we-really-be-creating-the-value-foo-in-our-user-s-production-databases which isn't the ideal placeholder.

[mtbc]

My suggestion is that we have this script delete only those orphan non-sharable annotations that have a NULL namespace or one starting with openmicroscopy.org/. Any thoughts or objections?

[chris-allan]

My gut feeling is that we leave this as an optional part of the database upgrade, document it as such and allow people to run it at their own risk. I don't like the premise of deleting rows in people's databases.

[mtbc]

Okay, happy to go with that unless anyone thinks otherwise.