Skip to content

FIX default_sort has no associated index#11680

Closed
lekoala wants to merge 1 commit intosilverstripe:5.3from
lekoala:patch-70
Closed

FIX default_sort has no associated index#11680
lekoala wants to merge 1 commit intosilverstripe:5.3from
lekoala:patch-70

Conversation

@lekoala
Copy link
Copy Markdown
Contributor

@lekoala lekoala commented Apr 9, 2025

Description

See issue for context

This PR adds a new index that matches the default_sort

This index should help many queries, also when filtering by surname/firstname (eg: in autocomplete scenarios). With this index, it means it actually better to filter against Surname/FirstName rather than just Surname

Manual testing steps

Simply use Member::get()->first()
without the index, the db engine may be using filesort due to default_sort being used

EXPLAIN SELECT * FROM Member ORDER BY Surname ASC, FirstName ASC LIMIT 1

would show "using filesort" on all records of the table

with the index, it shouldn't, and it will fetch the first record from the index directly

for filters, queries like

EXPLAIN SELECT * FROM Member 
WHERE Surname LIKE 'test%' AND FirstName LIKE 'test%'
ORDER BY Surname ASC, FirstName ASC LIMIT 1

# or even
EXPLAIN SELECT * FROM Member 
WHERE Surname LIKE 'test%'
ORDER BY Surname ASC, FirstName ASC LIMIT 1

will now be using the index as well resulting in much faster queries

Issues

#11674

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
    • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

@lekoala
Copy link
Copy Markdown
Contributor Author

lekoala commented Apr 9, 2025

Looking deeper into this, it seems that
#7559

should already be dealing with that, but it doesn't seem to be working as expected

it's even mentionned in the docs
https://docs.silverstripe.org/en/5/developer_guides/model/indexes/

All fields used in default_sort configuration

not sure what's going on here

@lekoala
Copy link
Copy Markdown
Contributor Author

lekoala commented Apr 9, 2025

ok so after a bit of testing, currently the default sort is split into two indexes, one for Surname and one for FirstName here

protected function buildSortDatabaseIndexes($class)
{
$sort = Config::inst()->get($class, 'default_sort', Config::UNINHERITED);
$indexes = [];
if ($sort && is_string($sort)) {
$sort = preg_split('/,(?![^()]*+\\))/', $sort ?? '');
foreach ($sort as $value) {
try {
list ($table, $column) = $this->parseSortColumn(trim($value ?? ''));
$table = trim($table ?? '', '"');
$column = trim($column ?? '', '"');
if ($table && strtolower($table ?? '') !== strtolower(DataObjectSchema::tableName($class) ?? '')) {
continue;
}
if ($this->databaseField($class, $column, false)) {
$indexes[$column] = [
'type' => 'index',
'columns' => [$column],
];
}
} catch (InvalidArgumentException $e) {
}
}
}
return $indexes;

Where in the current state

This works "mostly" well when using a where clause, eg

image

there is still a filesort, but since it's very likely on a small resultset, it's not noticeable

No where clause and a limit

these indexes seem to be ignored when there is no where clause (could be db engine dependent, I'm testing on mysql 8.0)

No multi column index => SELECT * FROM Member ORDER BY Surname ASC, FirstName ASC LIMIT 1
image

With multi column index => SELECT * FROM Member ORDER BY Surname ASC, FirstName ASC LIMIT 1
image

Where and multiple indexes

when there are multiple indexes (the two generated from the default_sort, and the one I added), the engine is not always super smart

image

vs

EXPLAIN SELECT * FROM Member FORCE INDEX(SortedName)
WHERE FirstName LIKE 'test%' AND Surname LIKE 'test%'
ORDER BY Surname ASC, FirstName ASC LIMIT 1

image

this is not the first time I have issues with combined indexes that are not as fast as expected. I haven't looked too much into it, but things may get even worse when joining on a table (a Member with many_many other Members)

I'm thinking that maybe splitting columns from the default_sort is not always great and it would be much better to actually use a multi column index

some documentation on multi columns indexes: https://dev.mysql.com/doc/refman/8.4/en/multiple-column-indexes.html

@GuySartorelli
Copy link
Copy Markdown
Member

GuySartorelli commented Jun 4, 2025

Closing in favour of #11747
This would have probably had to target 6 anyway since it's changing the database schema - which is a bit of a grey zone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants