[imaging_browser] Fix inefficient query for first acquisition date

[driusan]

The mri_acquisition_date table dropped in LORIS 22.0, and replaced by
a JOIN to the parameter_file table in order to look up the header
directly. However, ths query is missing a clause to restrict it to
only look up the acquisition_date header, resulting in an
unconditional join to parameter_file. Due to the size of the table
in existing projects, this can make the imaging browser menu page
take minutes to load.

This adds a where clause to restrict it to joining the acquisition_date
header, making the load time of the module usable again.

[driusan]

@cmadjar I guessed at the parameter name that it was supposed to be using. Can you verify that this is correct (and test it, I guess..) so we can push out a bug fix and push the branch forward? I don't think the imaging_browser is currently useable for projects with a moderate amount of data in either LORIS 22 or 23.

[cmadjar]

@driusan I cannot test it on my computer because the follow bug is present in 22 and I have -defaced scans. So I guess the fix from the following PR should also go into 22:

#6668

[driusan]

@cmadjar can you cherry-pick this PR into the 23 branch to test it, maybe?

[cmadjar]

@driusan I just thought of one thing. If the project insert images where all dates were removed then the query would fail because there would not be an acquisition_date row in the parameter_type table or any date header.

I just checked openpreventad and acquisition_date does not exist in the parameter_type table.

Not sure how to best tackle this...

[driusan]

@cmadjar what does openpreventad currently show as the first acq date in the imaging browser menu?

[cmadjar]

Nothing but openpreventad is running LORIS 20.1 so we the mri_acquisition_dates with a bunch of NULL values associated to the SessionIDs

[driusan]

In that case I think we'll need to turn this in to a LEFT JOIN to get similar behaviour

[driusan]

I've done some testing on this this morning and I'm not able to get it to work efficiently with a LEFT JOIN. Adding a (pt.Name='acquisition_date' OR pt.Name IS NULL) clause makes the performance plummet to unusable again (pt.Name is already indexed). I've tried different variations of adding AND/OR in the WHERE clause vs moving things into the ON clause of the JOIN, using IN instead of OR, and nothing seems to help.

Surprisingly the best I've been able to do in a way that handles the null case correctly was by moving it to a subselect, but that still takes ~10s to load with only about 2000 sessions in my test database, it won't scale to larger data sets.

I'll add something to discuss at the next LORIS meeting.

[driusan]

@cmadjar looking at the query again, won't it already filter out those rows if the header is missing since it's a JOIN and not a LEFT JOIN?

[cmadjar]

@driusan indeed, it will filter out those rows IF you add the AND pt.Name='acquisition_date' to the WHERE part of the query. That was why I mentioned that sometimes this information is not available hence it would remove rows from the list if we add the restriction in the where.

I am still unclear though how that query worked and how it could return an actual date in the data table for the First Acquisition with the current code... I really don't get it...

[driusan]

I am still unclear though how that query worked [..]

I don't think it does. I just checked out v23.0.0 (I can't verify v22 because my database tables aren't setup for it..) and it takes 3-4 minutes to load and then the column is empty for all the rows. (I sorted both forward and reverse on the first acq date column to check.)

[driusan]

Update from LORIS meeting: We're going to go with this fix for 22 and 23, and add a Acquisition Date column to the files table (and update the import scripts) to handle this more robustly for all data types.

[driusan]

Issues created for the work that needs to be done on the main branch to properly fix this.