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 a prefix index to filecache.path #26070

Merged
merged 1 commit into from
Mar 19, 2021
Merged

add a prefix index to filecache.path #26070

merged 1 commit into from
Mar 19, 2021

Conversation

icewind1991
Copy link
Member

The reason that filecache.path hasn't had an index added is the mysql limitation of ~1kb for indexeded fields,
which is to small for the path, however mysql supports indexing only the first N bytes of a column instead of the entire column,
allowing us to add an index even if the column is to long.

Because the index doesn't cover the entire column it can't be used in all situations where a normal index would be used, but it does cover the path like 'folder/path/%' queries that are used in various places.

Sqlite and Postgresql don't support prefix indexes, but they also don't have the 1kb limit and DBAL handles the differences in index creation.

Signed-off-by: Robin Appelman robin@icewind.nl

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Mar 11, 2021
@icewind1991 icewind1991 added this to the Nextcloud 22 milestone Mar 11, 2021
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

🐘

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@MorrisJobke
Copy link
Member

Error while trying to initialise the database: An exception occurred while executing a query: SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was too long; max key length is 3072 bytes

@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 19, 2021
@icewind1991 icewind1991 added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Mar 19, 2021
The reason that `filecache.path` hasn't had an index added is the mysql limitation of ~1kb for indexeded fields,
which is to small for the `path`, however mysql supports indexing only the first N bytes of a column instead of the entire column,
allowing us to add an index even if the column is to long.

Because the index doesn't cover the entire column it can't be used in all situations where a normal index would be used, but it does cover the `path like 'folder/path/%'` queries that are used in various places.

Sqlite and Postgresql don't support prefix indexes, but they also don't have the 1kb limit and DBAL handles the differences in index creation.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
Copy link
Member Author

Fixed the index definition

@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 19, 2021
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 2cae541 into master Mar 19, 2021
@MorrisJobke MorrisJobke deleted the path-prefix-index branch March 19, 2021 19:21
@MorrisJobke
Copy link
Member

Cc @nickvergessen @ChristophWurst @rullzer because of the discussion in chat

@icewind1991
Copy link
Member Author

/backport to stable21

@icewind1991
Copy link
Member Author

/backport to stable20

@backportbot-nextcloud
Copy link

The backport to stable20 failed. Please do this backport manually.

@rullzer
Copy link
Member

rullzer commented Mar 30, 2021

@icewind1991 backport to stable20 failed. still needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants