Skip to content

Commit 30d01b6

Browse files
authored
Merge pull request #37560 from nextcloud/backport/36097/stable25
[stable25] extend path-prefix optimizer to remove all cases of path_hash= when encountering a path prefix filter
2 parents daa16cc + 5e0c98b commit 30d01b6

3 files changed

Lines changed: 58 additions & 13 deletions

File tree

lib/private/Files/Search/QueryOptimizer/PathPrefixOptimizer.php

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,49 @@
2929
use OCP\Files\Search\ISearchOperator;
3030

3131
class PathPrefixOptimizer extends QueryOptimizerStep {
32-
public function processOperator(ISearchOperator &$operator) {
33-
// normally the `path = "$prefix"` search query part of the prefix filter would be generated as an `path_hash = md5($prefix)` sql query
32+
private bool $useHashEq = true;
33+
34+
public function inspectOperator(ISearchOperator $operator): void {
35+
// normally any `path = "$path"` search filter would be generated as an `path_hash = md5($path)` sql query
3436
// since the `path_hash` sql column usually provides much faster querying that selecting on the `path` sql column
3537
//
36-
// however, since we're already doing a filter on the `path` column in the form of `path LIKE "$prefix/%"`
38+
// however, if we're already doing a filter on the `path` column in the form of `path LIKE "$prefix/%"`
3739
// generating a `path = "$prefix"` sql query lets the database handle use the same column for both expressions and potentially use the same index
40+
//
41+
// If there is any operator in the query that matches this pattern, we change all `path = "$path"` instances to not the `path_hash` equality,
42+
// otherwise mariadb has a tendency of ignoring the path_prefix index
43+
if ($this->useHashEq && $this->isPathPrefixOperator($operator)) {
44+
$this->useHashEq = false;
45+
}
46+
47+
parent::inspectOperator($operator);
48+
}
49+
50+
public function processOperator(ISearchOperator &$operator) {
51+
if (!$this->useHashEq && $operator instanceof ISearchComparison && $operator->getField() === 'path' && $operator->getType() === ISearchComparison::COMPARE_EQUAL) {
52+
$operator->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, false);
53+
}
54+
55+
parent::processOperator($operator);
56+
}
57+
58+
private function isPathPrefixOperator(ISearchOperator $operator): bool {
3859
if ($operator instanceof ISearchBinaryOperator && $operator->getType() === ISearchBinaryOperator::OPERATOR_OR && count($operator->getArguments()) == 2) {
3960
$a = $operator->getArguments()[0];
4061
$b = $operator->getArguments()[1];
41-
if ($a instanceof ISearchComparison && $b instanceof ISearchComparison && $a->getField() === 'path' && $b->getField() === 'path') {
42-
if ($a->getType() === ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE && $b->getType() === ISearchComparison::COMPARE_EQUAL
43-
&& $a->getValue() === SearchComparison::escapeLikeParameter($b->getValue()) . '/%') {
44-
$b->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, false);
45-
}
46-
if ($b->getType() === ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE && $a->getType() === ISearchComparison::COMPARE_EQUAL
47-
&& $b->getValue() === SearchComparison::escapeLikeParameter($a->getValue()) . '/%') {
48-
$a->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, false);
49-
}
62+
if ($this->operatorPairIsPathPrefix($a, $b) || $this->operatorPairIsPathPrefix($b, $a)) {
63+
return true;
5064
}
5165
}
66+
return false;
67+
}
5268

53-
parent::processOperator($operator);
69+
private function operatorPairIsPathPrefix(ISearchOperator $like, ISearchOperator $equal): bool {
70+
return (
71+
$like instanceof ISearchComparison && $equal instanceof ISearchComparison &&
72+
$like->getField() === 'path' && $equal->getField() === 'path' &&
73+
$like->getType() === ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE && $equal->getType() === ISearchComparison::COMPARE_EQUAL
74+
&& $like->getValue() === SearchComparison::escapeLikeParameter($equal->getValue()) . '/%'
75+
);
5476
}
5577
}

lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ public function __construct(
3838
}
3939

4040
public function processOperator(ISearchOperator $operator) {
41+
foreach ($this->steps as $step) {
42+
$step->inspectOperator($operator);
43+
}
4144
foreach ($this->steps as $step) {
4245
$step->processOperator($operator);
4346
}

lib/private/Files/Search/QueryOptimizer/QueryOptimizerStep.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,26 @@
2727
use OCP\Files\Search\ISearchOperator;
2828

2929
class QueryOptimizerStep {
30+
/**
31+
* Allow optimizer steps to inspect the entire query before starting processing
32+
*
33+
* @param ISearchOperator $operator
34+
* @return void
35+
*/
36+
public function inspectOperator(ISearchOperator $operator): void {
37+
if ($operator instanceof ISearchBinaryOperator) {
38+
foreach ($operator->getArguments() as $argument) {
39+
$this->inspectOperator($argument);
40+
}
41+
}
42+
}
43+
44+
/**
45+
* Allow optimizer steps to modify query operators
46+
*
47+
* @param ISearchOperator $operator
48+
* @return void
49+
*/
3050
public function processOperator(ISearchOperator &$operator) {
3151
if ($operator instanceof ISearchBinaryOperator) {
3252
foreach ($operator->getArguments() as $argument) {

0 commit comments

Comments
 (0)