Skip to content

Conversation

@ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Mar 12, 2025

Summary

While working on #51438 I noticed that replacement indexes are not created if the old indexes don't exist. This can happen in the following deployment

  1. Table is created but without index
  2. Optional index is added but admin never applies it
  3. The previously optional index is replaced by another one

Before: old and new index were missing
After: new index is added

The setup check has already reported the affected indices as missing, but they were never added.

TODO

  • Make the changes
  • Test the changes

Checklist

@ChristophWurst ChristophWurst self-assigned this Mar 12, 2025
@ChristophWurst ChristophWurst requested a review from a team as a code owner March 12, 2025 12:23
@ChristophWurst ChristophWurst requested review from artonge, come-nc and nfebe and removed request for a team March 12, 2025 12:23
@ChristophWurst
Copy link
Member Author

/backport to stable31

@ChristophWurst
Copy link
Member Author

/backport to stable30

$table = $schema->getTable($toReplaceIndex['tableName']);
// If none of the previous indices existed we have to add the index
if (array_filter($toReplaceIndex['oldIndexNames'], static fn (string $indexName) => $table->hasIndex($indexName)) === []) {
$missingIndices[] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw an entry in the Nextcloud log saying "Undefined array key "dropUnnamedIndex" at /knickrehm/www/cloud/core/Command/Db/AddMissingIndices.php#79" after applying this fix. From what I see, this array should contain a value for the key "dropUnnamedIndex" or 79 should not expect a value to exist for that key.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Not sure I get it, there was a code path for when all old names existed, you added one for when non of them exists, but there is still an issue if only some of them exists, no?

Why not simply ignore the existing, add the new one and then drop the old ones if they exist?

@ChristophWurst
Copy link
Member Author

Actually fair point. I'll look into that!

@ChristophWurst
Copy link
Member Author

@come-nc thanks a lot for the suggestion. The code is a lot simpler now!

@ChristophWurst ChristophWurst requested a review from come-nc April 15, 2025 17:30
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the fix/create-missing-replacement-indexes branch from 063c77f to 29161c0 Compare April 17, 2025 15:59
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 17, 2025
@AndyScherzinger AndyScherzinger added this to the Nextcloud 32 milestone Apr 23, 2025
@AndyScherzinger AndyScherzinger merged commit 2c8c036 into master Apr 23, 2025
200 of 208 checks passed
@AndyScherzinger AndyScherzinger deleted the fix/create-missing-replacement-indexes branch April 23, 2025 22:49
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: database Database related DB performance 🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

missing indices after activating Mail in NC-AIO

6 participants