Skip to content

refactor: Rename tables & columns to snake_case - #797

Open
MaienM wants to merge 1 commit into
DialmasterOrg:devfrom
MaienM:feature/normalize-db-names
Open

refactor: Rename tables & columns to snake_case#797
MaienM wants to merge 1 commit into
DialmasterOrg:devfrom
MaienM:feature/normalize-db-names

Conversation

@MaienM

@MaienM MaienM commented Aug 30, 2026

Copy link
Copy Markdown

As discussed in Discord. This changes all table & column names to be lowercase/snake_cased (with the exception of the Sequelize metadata table which we don't manually interact with in any way).

This is done in part because it'll make adding PostgreSQL support a bit easier (since that automatically lowercases any such names unless quoted), but it also makes things more consistent since new tables/columns already follow this format.

Some notes:

  • The underscored option for models will automatically use snake_cased names for the camelCased attributes, so the field key is only specified for the two attributes that would have a you_tube_... column following this format as that seemed undesirable.
  • The underscored option for models is also applied to the createdAt and updatedAt attributes that are added by the timestamps option.
  • I have also tested this on a database that was started with --lower_case_table_names=1 and verified that worked as expected.

The raw queries are the majority of the changes here. I've done my best to trigger the code paths containing these to verify them on a running instance, but I'm sure I've missed some (permutations) so these should be reviewed carefully.

@dialmaster dialmaster left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, but I found a few things that need to be addressed:

  1. Without ANSI_QUOTES in sql_mode (we don't set it), MySQL/MariaDB treats "..." as a string literal, not an identifier. Using it in the alias definition (AS "timeCreated") is fine, but referencing it as ORDER BY "timeCreated" DESC sorts by a constant string (i.e. doesn't sort at all), and HAVING "timeCreated" IS NOT NULL is always true. No error; it just silently stops sorting/filtering.

This hits autoRemovalQueries.getRecentVideoIds / getChannelKeepRecentIds (the keep-recent guards would pick N arbitrary videos instead of the N most recent, so auto-removal could delete exactly the videos they're supposed to protect) and videoDeletionModule.getOldestVideos (free-space cleanup would delete an arbitrary subset instead of the oldest). The other two candidate queries have the same pattern but no LIMIT, so it's mostly harmless there.

It's also not portable in the other direction: Postgres doesn't allow output-column aliases in HAVING at all. HAVING MAX(<expr>) IS NOT NULL + ORDER BY MAX(<expr>) DESC works on both dialects; a lowercase unquoted alias (AS time_created ... ORDER BY time_created) also works for the ORDER BY cases. The updated tests assert the quoted strings, so they need to change along with it.

  1. In videosModule.js the query now returns youtube_channel_name, but the consumer a few lines down still reads row.youTubeChannelName, so channels that only exist in the videos table silently drop out of the Videos page filter dropdown. Easiest fix is AS "youTubeChannelName" like the other queries. (The test mocks that query's result as [], which is why nothing caught it.)

  2. migrations/20260830191259-repair-jobs-uuid-fk-collation.js was merged to dev after you branched, and it runs ALTERs with hardcoded Jobs/JobVideos names (via migrations/lib/jobsUuidCollation.js). Migrations execute in filename-timestamp order and your ...101917 sorts before ...191259, so on a DB that applies both in one boot the rename runs first, and the repair's ALTER TABLE Jobs ... then fails on any database that still needs it (table names are case-sensitive on Linux). Since the repair is already merged I can't re-timestamp that one, so: can you rebase on dev and bump the rename migration's timestamp to sort after 191259? Then every migration sees the names it was written against, on both fresh and upgraded DBs.

  3. docs/TROUBLESHOOTING.md still has manual SQL using the old JobVideos/JobVideoDownloads names.

@MaienM
MaienM force-pushed the feature/normalize-db-names branch from 5d8f73d to 1f475ae Compare August 31, 2026 10:54
@MaienM

MaienM commented Aug 31, 2026

Copy link
Copy Markdown
Author

1. Changing the name to time_created would definitely be the easiest way to handle this, but since this column is mentioned in the docs for return of the function I assumed the name was part of the contract of this function and you would want it to remain as-is. I've gone ahead and just removed the quotes around this name for now since that works fine everywhere in MySQL and PostgreSQL isn't in scope for this PR.

4. I had noticed these uses of these table names but because these troubleshooting sections pertain to issues with migrations which will run before this rename I though it would be best to not touch these since any user needing this section will have a database in which these items have not yet been renamed.

@MaienM
MaienM requested a review from dialmaster August 31, 2026 17:09
@dialmaster

Copy link
Copy Markdown
Collaborator

I went through this pretty carefully since it renames basically every table an existing install has.
I ran it against a real pre-existing DB and it came through clean, FKs survived the renames, no camelCase columns left behind, and schema validation passes. A few additional things I think we still need to fix before it gets merged though:

  1. External DBs with lower_case_table_names=2 (native macOS MariaDB, mostly) will crash on the table renames. The guard handles lctn=1 fine, but on lctn=2 the stored name is still Jobs, so the rename runs and dies with "table already exists" (I reproduced that exact error under lctn=1 with the guard removed). A two-step rename through a temp name works on every setting.
  2. TROUBLESHOOTING.md line 454 still says ALTER TABLE Videos DROP COLUMN media_type;, which fails on a post-rename DB. (The collation sections above it are fine as-is; those errors happen before the rename runs.)
  3. jobvideo.js and jobvideodownload.js still have references: { model: 'Jobs' } / 'Videos'.
  4. Can we get a test for the migration? We have them for the other delicate ones and this is the riskiest one yet.

Overall... this looks pretty good, but I'm always wary about rolling out something like this since it if anything goes wrong it will completely break Youtarr for existing users and make fixing it a difficult/manual process :)

@MaienM
MaienM force-pushed the feature/normalize-db-names branch from 1f475ae to 2abf3d8 Compare August 31, 2026 22:57
@MaienM

MaienM commented Aug 31, 2026

Copy link
Copy Markdown
Author

I've added tests that explicitly cover all 3 LCTN variants and have adjusted the migration to work in all these cases (which includes the two-step rename but also adds a check whether the table exists in the column renames as otherwise downgrading from a partial migration might not work).

I understand you being careful about this change and I appreciate the thorough reviews & you testing this on a much more representative dataset than I possess.

I've also include an SQL script below that one can use to rollback this migration manually so that we have a fallback in case any users do end up having issues. For someone with a partial migration this'll probably throw some errors for the items that already have the old names but that's fine since that just means that those statements weren't needed anyway for that database. We could include this in the troubleshooting docs if you'd like, but I'll leave that decision up to you.

ALTER TABLE `apikeys` RENAME TO `apikeys-tmp`;
ALTER TABLE `apikeys-tmp` RENAME TO `ApiKeys`;
ALTER TABLE `jobvideodownloads` RENAME TO `jobvideodownloads-tmp`;
ALTER TABLE `jobvideodownloads-tmp` RENAME TO `JobVideoDownloads`;
ALTER TABLE `jobvideos` RENAME TO `jobvideos-tmp`;
ALTER TABLE `jobvideos-tmp` RENAME TO `JobVideos`;
ALTER TABLE `jobs` RENAME TO `jobs-tmp`;
ALTER TABLE `jobs-tmp` RENAME TO `Jobs`;
ALTER TABLE `sessions` RENAME TO `sessions-tmp`;
ALTER TABLE `sessions-tmp` RENAME TO `Sessions`;
ALTER TABLE `videos` RENAME TO `videos-tmp`;
ALTER TABLE `videos-tmp` RENAME TO `Videos`;

ALTER TABLE `channels` CHANGE `last_fetched_by_tab` `lastFetchedByTab` TEXT;
ALTER TABLE `channelvideos` CHANGE `published_at` `publishedAt` VARCHAR(255) DEFAULT NULL;
ALTER TABLE `Jobs` CHANGE `job_type` `jobType` VARCHAR(255) NOT NULL;
ALTER TABLE `Jobs` CHANGE `time_created` `timeCreated` DATETIME NOT NULL;
ALTER TABLE `Jobs` CHANGE `time_initiated` `timeInitiated` DATETIME NOT NULL;
ALTER TABLE `media_server_users` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `media_server_users` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;
ALTER TABLE `playlist_sync_state` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `playlist_sync_state` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;
ALTER TABLE `playlists` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `playlists` CHANGE `last_fetched` `lastFetched` DATETIME DEFAULT NULL;
ALTER TABLE `playlists` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;
ALTER TABLE `playlistvideos` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `playlistvideos` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;
ALTER TABLE `Sessions` CHANGE `created_at` `createdAt` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
ALTER TABLE `Sessions` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
ALTER TABLE `subfolders` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `subfolders` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;
ALTER TABLE `video_watch_status` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `video_watch_status` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;
ALTER TABLE `Videos` CHANGE `audio_file_path` `audioFilePath` VARCHAR(500) DEFAULT NULL;
ALTER TABLE `Videos` CHANGE `audio_file_size` `audioFileSize` BIGINT(20) DEFAULT NULL;
ALTER TABLE `Videos` CHANGE `file_path` `filePath` VARCHAR(500) DEFAULT NULL;
ALTER TABLE `Videos` CHANGE `file_size` `fileSize` BIGINT(20) DEFAULT NULL;
ALTER TABLE `Videos` CHANGE `original_date` `originalDate` VARCHAR(255) DEFAULT NULL;
ALTER TABLE `Videos` CHANGE `youtube_channel_name` `youTubeChannelName` VARCHAR(255) NOT NULL;
ALTER TABLE `Videos` CHANGE `youtube_video_name` `youTubeVideoName` VARCHAR(255) NOT NULL;
ALTER TABLE `Videos` CHANGE `youtube_id` `youtubeId` VARCHAR(255) NOT NULL;
ALTER TABLE `watch_status_sync_cursors` CHANGE `created_at` `createdAt` DATETIME NOT NULL;
ALTER TABLE `watch_status_sync_cursors` CHANGE `updated_at` `updatedAt` DATETIME NOT NULL;

DELETE FROM `SequelizeMeta` WHERE `name` = '20260830201917-lowercased-table-column-names.js'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants