Differential D6465
[keyserver] Add thread column to uploads table Authored by rohan on Jan 30 2023, 7:26 AM. Tags None Referenced Files
Subscribers Tokens
Details Attaching a thread column to the uploads table will important so we can associate each upload with a thread. This will allow us to later query media that was uploaded in a specific thread. https://linear.app/comm/issue/ENG-2862/modify-the-uploads-table-to-include-a-thread-column (uploads table modification) https://linear.app/comm/issue/ENG-2883/create-a-script-to-retroactively-populate-the-thread-column-in-the (retroactively populate table) Restart keyserver and check the tables on TablePlus for uploads to check and confirm that there is a new thread column. 
Diff Detail
Event TimelineComment Actions Not 100% sure if this is necessary but should we also add thread to the ALTER statement for the uploads table Comment Actions
Nvm, I think we only do this if we made thread and INDEX Comment Actions We need an index on thread if we plan to query on it We also need a script to populate the column, not sure if that's meant to be a separate diff Comment Actions Please think this through... you're not adding the index in the migration. This also wasn't addressed:
Comment Actions Ah right, this slipped my mind sorry about that. To address the script, do you mean retroactively populate for existing media? This is the linear task for that per @atul's suggestion on Linear https://linear.app/comm/issue/ENG-2883/create-a-script-to-retroactively-populate-the-thread-column-in-the Comment Actions Add the index into the migration following the statement to add the thread column. Followed migration #9 to confirm syntax with multipleStatements, and restarted keyserver to check that it starts up fine. Comment Actions
Yeah, that's what I mean. Except instead of a script it should just be a migration. Could just be migration 15, or you could introduce a subsequent migration later. Note that it's important that we land the code that will populate the thread column for new uploads at the same time as (or earlier than) we land the migration that will populate the thread column for existing uploads – otherwise we might get some uploads that are missing thread. Comment Actions
(This would only be possible if you wait to land this until you have that other migration code ready to land as well) Comment Actions Include the UPDATE statement to retroactively update all entries in the uploads table to have a corresponding thread. Tested by doing the following:
An additional update I added in this revision is IF NOT EXISTS to ADD INDEX IF NOT EXISTS thread (thread); to prevent anyone who ran the migration (mainly me, but if anyone tested this locally as Before:  After:  Comment Actions Requesting review again with the new changes to retroactively populate the thread column for existing uploads. Comment Actions Please don't land this until you're ready to land a separate diff (presumably D6482) that updates keyserver to populate this column for new rows... otherwise we'll have a set of rows in the table without the thread column populated
Comment Actions Combine the ALTER TABLE queries and re-do the the testing as mentioned in the previous revision. Confirmed migration 15 succeeded. was in my console.
Comment Actions Reduce line length. Set my DB version back to 14 and restarted keyserver for good measure | ||||||||||||||||||||||||||||||||||||||||