Load only the necessary work fields when displaying series
Description
We sometimes load works ordered by their positions in a series like this:
This generates a SQL query that includes every single column from the works table, which is bad for 2 reasons:
The query is slower for getting unnecessary data.
We run migrations to remove seemingly unused columns from the works table, but existing Rails processes will still try to SELECT those columns, leading to errors on series and works in series. This did happen when we deployed AO3-5905, and we fixed the errors by restarting all Rails processes:
For reference, the errors we got were like:
Instances of this query:
Displaying the next/previous work links in a work meta: we need the work IDs, plus the fields used in the visible? check (posted, hidden_by_admin, and restricted).
Displaying the series: we do need everything for work blurbs. Not sure if we can easily limit the fields to load, but I'm noting it here anyway.
How to test:
Check that you can view a series and reorder the works in it without errors.
Check that you can view the blurbs of works in a series, with the expected link to the series, and without errors.
Check that you can view the meta boxes of works in a series, with the expected links to the series + the next/previous works, and without errors.
Activity
Two thoughts:
Calling includes(:work) alone doesn’t make rails select both tables in a single query. The joint query is caused when you have includes(:work).references(:works), or includes(:work) combined with some where queries that reference the works table (which causes an implicit call to references(:works)). Neither of these occurs in SeriesController#manage, so I don’t believe it needs to be modified to fix the issue with migrations.
The code in the work meta calls Work#visible?, which uses a couple of different fields from the Work object: posted, hidden_by_admin, and restricted. Retrieving the id alone will make the visibility check fail for everyone except admins and the owner of the work, because it’ll mean the Work looks like a draft.
Thanks, I updated the issue. I'm not sure we can fix SeriesController#show, but we can certainly fix the query for work metas.
It requires more work, rather than less, but … maybe it’d work to either avoid where(works: { posted: true }) and just rely on visible? for the permission/visibility check, or to use joins(:work).preload(:work) instead of includes(:work).
Or just flip the query around a little and do Work.joins(:serial_works).merge(@series.serial_works).select("works.*, serial_works.position AS position”), since I think position is the only field required from serial_works. (And I’m not even sure it’s required.)
Okay, I’m a little confused on this one but:
Created new Series with numbered works
Successfully reordered said works
While logged in as testy was able to view the blurbs of the series works with the correct links to correct works
While logged in as testy was able to view the Series information on the works pages & correctly move through the works in the correct (reordered) order
Logged out of testy & was able to view blurbs with correct links & move through Series using the Next/Previous buttons in the correct (reordered) order.
I’m not 100% sure if that’s what was being asked for, but at the very least I can confirm that reordering works?
Created a series, was able to see work blurbs and navigate to next/previous works correctly without errors
Reordered the series without errors and was able to correct navigate between works with no errors.
Viewed an older series, correctly able to view series inforamtion and all links worked as expected.
