Replace notes(userId) index with (userId, id) #28

Merged

sugar merged 4 commits from sugar/forkey:replace-notes-user-id-index-with-user-id-id into main 2025-02-23 13:05:55 +00:00

Conversation 4 Commits 4 Files changed 2 +12

sugar commented 2025-01-11 21:37:03 +00:00

Owner

Copy link

What

This improves performance of queries searching posts by a given user, optionally with a provided range of ids (pagination), and then using ORDER BY to sort posts by date, with a provided limit.

Examples of such queries include: viewing posts by a given user, as well as checking when the last post by a given user was written for Mastodon API.

Why

Making Mastodon API more performant, as it will look up the date of most recent post a lot (using index-only scan). Also, this will help with performance of users/notes Misskey API.

Iceshrimp already did that change before: 2efdbae42e.

Without this index, the database essentially has to fetch all posts by a given user to be able to determine what were the most recent posts.

See https://use-the-index-luke.com/sql/partial-results/top-n-queries.

Additional info (optional)

Checklist

• Read the contribution guide
• Test working in a local environment
• (If needed) Add story of storybook
• (If needed) Update CHANGELOG.md
• (If possible) Add tests

<!-- ℹ お読みください / README PRありがとうございます！ PRを作成する前に、コントリビューションガイドをご確認ください: Thank you for your PR! Before creating a PR, please check the contribution guide: https://github.com/misskey-dev/misskey/blob/develop/CONTRIBUTING.md --> ## What <!-- このPRで何をしたのか？ どう変わるのか？ --> <!-- What did you do with this PR? How will it change things? --> This improves performance of queries searching posts by a given user, optionally with a provided range of `id`s (pagination), and then using ORDER BY to sort posts by date, with a provided limit. Examples of such queries include: viewing posts by a given user, as well as checking when the last post by a given user was written for Mastodon API. ## Why <!-- なぜそうするのか？ どういう意図なのか？ 何が困っているのか？ --> <!-- Why do you do it? What are your intentions? What is the problem? --> Making Mastodon API more performant, as it will look up the date of most recent post a lot (using index-only scan). Also, this will help with performance of `users/notes` Misskey API. Iceshrimp already did that change before: https://iceshrimp.dev/iceshrimp/iceshrimp/commit/2efdbae42e75992965069f6e7601a724cc5da9a1. Without this index, the database essentially has to fetch all posts by a given user to be able to determine what were the most recent posts. See https://use-the-index-luke.com/sql/partial-results/top-n-queries. ## Additional info (optional) <!-- テスト観点など --> <!-- Test perspective, etc --> ## Checklist - [ ] Read the [contribution guide](https://github.com/misskey-dev/misskey/blob/develop/CONTRIBUTING.md) - [ ] Test working in a local environment - [ ] (If needed) Add story of storybook - [ ] (If needed) Update CHANGELOG.md - [ ] (If possible) Add tests

leah commented 2025-01-14 20:44:28 +00:00

Owner

Copy link

The iceshrimp change also changes another file and adds @Index("IDX_note_userId_id", ["userId", "id"]) to the top of Note. We have the MiNote class and I assume it would be good to put that there?

The iceshrimp change also changes another file and adds ` @Index("IDX_note_userId_id", ["userId", "id"])` to the top of Note. We have the MiNote class and I assume it would be good to put that there?

sugar force-pushed replace-notes-user-id-index-with-user-id-id from d1ab9aca8c to 83151ab37d 2025-01-14 21:06:41 +00:00 Compare

sugar force-pushed replace-notes-user-id-index-with-user-id-id from 83151ab37d to 93dba136ab 2025-01-17 15:29:11 +00:00 Compare

ashten added 1 commit 2025-01-19 04:41:31 +00:00

Merge branch 'main' into replace-notes-user-id-index-with-user-id-id 44edae531a

ashten added 1 commit 2025-01-19 06:06:22 +00:00

Merge branch 'main' into replace-notes-user-id-index-with-user-id-id 9f4a76a1d9

ashten commented 2025-01-19 06:35:17 +00:00

Owner

Copy link

this unit test is failing:

		test('フォルダが循環するような構造にできない(再帰的)', async () => {
			const folderA = (await api('drive/folders/create', {
				name: 'test',
			}, alice)).body;
			const folderB = (await api('drive/folders/create', {
				name: 'test',
			}, alice)).body;
			const folderC = (await api('drive/folders/create', {
				name: 'test',
			}, alice)).body;
			await api('drive/folders/update', {
				folderId: folderB.id,
				parentId: folderA.id,
			}, alice);
			await api('drive/folders/update', {
				folderId: folderC.id,
				parentId: folderB.id,
			}, alice);

			const res = await api('drive/folders/update', {
				folderId: folderA.id,
				parentId: folderC.id,
			}, alice);

			assert.strictEqual(res.status, 400);
		});

the translation is: Folders cannot be recursively structured.

it returns a 200 status code when it should be 400

this unit test is failing: ``` test('フォルダが循環するような構造にできない(再帰的)', async () => { const folderA = (await api('drive/folders/create', { name: 'test', }, alice)).body; const folderB = (await api('drive/folders/create', { name: 'test', }, alice)).body; const folderC = (await api('drive/folders/create', { name: 'test', }, alice)).body; await api('drive/folders/update', { folderId: folderB.id, parentId: folderA.id, }, alice); await api('drive/folders/update', { folderId: folderC.id, parentId: folderB.id, }, alice); const res = await api('drive/folders/update', { folderId: folderA.id, parentId: folderC.id, }, alice); assert.strictEqual(res.status, 400); }); ``` the translation is: `Folders cannot be recursively structured.` it returns a 200 status code when it should be 400

ashten commented 2025-01-19 07:30:03 +00:00

Owner

Copy link

im a bit unsure whats going on with that failing unit test. its now failing on another pull request even though it wasnt a few minutes ago

im a bit unsure whats going on with that failing unit test. its now failing on another pull request even though it wasnt a few minutes ago

ashten commented 2025-01-19 07:55:35 +00:00

Owner

Copy link

alright, i reran the unit tests, and its passing now

opened an issue: #44

alright, i reran the unit tests, and its passing now opened an issue: #44

ashten added 1 commit 2025-01-19 07:56:13 +00:00

Merge branch 'main' into replace-notes-user-id-index-with-user-id-id 884caa8c1e

leah approved these changes 2025-02-23 12:59:12 +00:00

leah left a comment

Owner

Copy link

Everything here looks good!

Everything here looks good!

sugar merged commit 598b88b142 into main 2025-02-23 13:05:55 +00:00

sugar referenced this pull request from a commit 2025-02-23 13:05:56 +00:00

Merge pull request 'Replace notes(userId) index with (userId, id)' (#28) from sugar/forkey:replace-notes-user-id-index-with-user-id-id into main

Sign in to join this conversation.

Reviewers

No reviewers

leah

Labels

Clear labels   

bug

Something is not working

  

customization

Change something to be more suitable for forkey

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

feature parity

Another fork has this feature, and it should be pulled into this fork.

  

help wanted

Need some help

  

high priority

  

invalid

Something is wrong

  

low priority

  

question

More information is needed

  

wontfix

This won't be fixed

No labels

bug

customization

duplicate

enhancement

feature parity

help wanted

high priority

invalid

low priority

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: woem.men/forkey#28

No description provided.