Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dav): add missing database index for dav_shares #46036

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

SebastianKrupinski
Copy link
Contributor

Summary

Added missing database indexes for dav_shares table, (resourceid, type) and (recourseid, access)

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Looks good otherwise.

@st3iny st3iny added this to the Nextcloud 30 milestone Jun 21, 2024
@ChristophWurst
Copy link
Member

Otherwise ... what?

Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good! Please squash into on single commit

@@ -235,6 +239,7 @@ public function registerHooks(HookManager $hm,

// Here we should recalculate if reminders should be sent to new or old sharees
});

Copy link
Member

Choose a reason for hiding this comment

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

🔥

@ChristophWurst
Copy link
Member

/backport to stable29

@@ -471,6 +471,9 @@ public function changeSchema(IOutput $output, \Closure $schemaClosure, array $op
]);
$table->setPrimaryKey(['id']);
$table->addUniqueIndex(['principaluri', 'resourceid', 'type', 'publicuri'], 'dav_shares_index');
// modified on 2024-6-21 to add performance improving indices on new instances
$table->addIndex(['resourceid', 'type'], 'dav_shares_resourceid_type');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if principaluri is used for searches, but if not, it could make sense to change the unique index instead of adding this new one:

$table->addUniqueIndex(['resourceid', 'type', 'principaluri', 'publicuri'], 'dav_shares_index');

…and access

Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>

Co-authored-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Sebastian Krupinski <165827823+SebastianKrupinski@users.noreply.github.com>

Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>

Co-authored-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Sebastian Krupinski <165827823+SebastianKrupinski@users.noreply.github.com>

Co-authored-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Sebastian Krupinski <165827823+SebastianKrupinski@users.noreply.github.com>

Co-authored-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Sebastian Krupinski <165827823+SebastianKrupinski@users.noreply.github.com>

Co-authored-by: Daniel <mail@danielkesselberg.de>
Signed-off-by: Sebastian Krupinski <165827823+SebastianKrupinski@users.noreply.github.com>

Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
@SebastianKrupinski SebastianKrupinski merged commit 380a253 into master Jun 26, 2024
165 checks passed
@SebastianKrupinski SebastianKrupinski deleted the fix/issue-46015 branch June 26, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Missing database index for dav_shares resourceid, type and access
7 participants