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

Add index for classification attribute #35459

Closed

Conversation

akhil1508
Copy link
Contributor

Summary

  • Added a migration to add an index for the "classification" column of the "calendarobjects" table

TODO

  • ...

Checklist

@akhil1508 akhil1508 force-pushed the dav/classification-index branch from 0621877 to e6c4d89 Compare November 28, 2022 13:57
@szaimen szaimen added the 3. to review Waiting for reviews label Nov 28, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Nov 28, 2022
@szaimen szaimen requested review from a team, PVince81, juliusknorr and CarlSchwan and removed request for a team November 28, 2022 14:01
@akhil1508 akhil1508 force-pushed the dav/classification-index branch from 4f56a96 to 25ff090 Compare November 28, 2022 16:44
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

Thank you 👍

apps/dav/lib/Migration/Version1025Date20221114151721.php Outdated Show resolved Hide resolved
apps/dav/lib/Migration/Version1025Date20221114151721.php Outdated Show resolved Hide resolved
apps/dav/lib/Migration/Version1025Date20221114151721.php Outdated Show resolved Hide resolved
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

You also need to bump the app version in apps/dav/appinfo/info.xml so that the migrations are executed.

@tcitworld
Copy link
Member

How long did creating the index took on your database? If it's too much we can move it to the dedicated missing indexes command (see for instance #33405)

@akhil1508
Copy link
Contributor Author

@tcitworld It took 14 seconds to create the index on a database with 1 million entries in the calendarobjects table

@akhil1508 akhil1508 force-pushed the dav/classification-index branch from 839c1d5 to 1d38939 Compare November 29, 2022 08:56
@akhil1508
Copy link
Contributor Author

@tcitworld Bumped from 1.25.0 to 1.25.1

@akhil1508 akhil1508 requested review from tcitworld and kesselb and removed request for PVince81, juliusknorr, CarlSchwan, miaulalala, tcitworld and kesselb November 29, 2022 10:53
@akhil1508 akhil1508 force-pushed the dav/classification-index branch from a856cbc to b08e885 Compare November 29, 2022 11:31
@akhil1508
Copy link
Contributor Author

@ALL it automatically requested review while I was trying to squash commits.. sorry :(

@kesselb I've tried squashing with different methods and none of them work as there are multiple merge commits from master in this one's history. Is it okay for you if I open PR in a new branch instead?

@susnux susnux removed their request for review October 19, 2023 16:39
@susnux
Copy link
Contributor

susnux commented Oct 20, 2023

@akhil1508 it should work with:

git remote add upstream https://github.com/nextcloud/server.git
git fetch upstream
git rebase -i upstream/master

then mark all of you commits as "s" (for squash) and it should work.

@akhil1508 akhil1508 force-pushed the dav/classification-index branch from beccaea to 604f141 Compare October 25, 2023 11:44
Signed-off-by: Akhil <akhil@e.email>
@akhil1508 akhil1508 force-pushed the dav/classification-index branch from 604f141 to 33fd502 Compare October 25, 2023 11:46
@akhil1508
Copy link
Contributor Author

@susnux I did as you suggested and force-pushed, now there are 33 changes instead of 3 changes 🤔

@tcitworld
Copy link
Member

@akhil1508 Is the « Allow edits by maintainers » checkbox checked on the sidebar? Seems I can't force push to fix things.

@akhil1508
Copy link
Contributor Author

@tcitworld Seems I cannot do that as this PR is made from a fork under an organization :(

As per https://github.com/orgs/community/discussions/5634

@tcitworld tcitworld mentioned this pull request Oct 25, 2023
4 tasks
@tcitworld
Copy link
Member

Replacement PR at #41111

@tcitworld tcitworld closed this Oct 25, 2023
@akhil1508
Copy link
Contributor Author

@tcitworld Thanks for the replacement PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants