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

[stable25] extend path-prefix optimizer to remove all cases of path_hash= when encountering a path prefix filter #37560

Merged
merged 1 commit into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 35 additions & 13 deletions lib/private/Files/Search/QueryOptimizer/PathPrefixOptimizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,49 @@
use OCP\Files\Search\ISearchOperator;

class PathPrefixOptimizer extends QueryOptimizerStep {
public function processOperator(ISearchOperator &$operator) {
// normally the `path = "$prefix"` search query part of the prefix filter would be generated as an `path_hash = md5($prefix)` sql query
private bool $useHashEq = true;

public function inspectOperator(ISearchOperator $operator): void {
// normally any `path = "$path"` search filter would be generated as an `path_hash = md5($path)` sql query
// since the `path_hash` sql column usually provides much faster querying that selecting on the `path` sql column
//
// however, since we're already doing a filter on the `path` column in the form of `path LIKE "$prefix/%"`
// however, if we're already doing a filter on the `path` column in the form of `path LIKE "$prefix/%"`
// generating a `path = "$prefix"` sql query lets the database handle use the same column for both expressions and potentially use the same index
//
// If there is any operator in the query that matches this pattern, we change all `path = "$path"` instances to not the `path_hash` equality,
// otherwise mariadb has a tendency of ignoring the path_prefix index
if ($this->useHashEq && $this->isPathPrefixOperator($operator)) {
$this->useHashEq = false;
}

parent::inspectOperator($operator);
}

public function processOperator(ISearchOperator &$operator) {
if (!$this->useHashEq && $operator instanceof ISearchComparison && $operator->getField() === 'path' && $operator->getType() === ISearchComparison::COMPARE_EQUAL) {
$operator->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, false);
}

parent::processOperator($operator);
}

private function isPathPrefixOperator(ISearchOperator $operator): bool {
if ($operator instanceof ISearchBinaryOperator && $operator->getType() === ISearchBinaryOperator::OPERATOR_OR && count($operator->getArguments()) == 2) {
$a = $operator->getArguments()[0];
$b = $operator->getArguments()[1];
if ($a instanceof ISearchComparison && $b instanceof ISearchComparison && $a->getField() === 'path' && $b->getField() === 'path') {
if ($a->getType() === ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE && $b->getType() === ISearchComparison::COMPARE_EQUAL
&& $a->getValue() === SearchComparison::escapeLikeParameter($b->getValue()) . '/%') {
$b->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, false);
}
if ($b->getType() === ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE && $a->getType() === ISearchComparison::COMPARE_EQUAL
&& $b->getValue() === SearchComparison::escapeLikeParameter($a->getValue()) . '/%') {
$a->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, false);
}
if ($this->operatorPairIsPathPrefix($a, $b) || $this->operatorPairIsPathPrefix($b, $a)) {
return true;
}
}
return false;
}

parent::processOperator($operator);
private function operatorPairIsPathPrefix(ISearchOperator $like, ISearchOperator $equal): bool {
return (
$like instanceof ISearchComparison && $equal instanceof ISearchComparison &&
$like->getField() === 'path' && $equal->getField() === 'path' &&
$like->getType() === ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE && $equal->getType() === ISearchComparison::COMPARE_EQUAL
&& $like->getValue() === SearchComparison::escapeLikeParameter($equal->getValue()) . '/%'
);
}
}
3 changes: 3 additions & 0 deletions lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public function __construct(
}

public function processOperator(ISearchOperator $operator) {
foreach ($this->steps as $step) {
$step->inspectOperator($operator);
}
foreach ($this->steps as $step) {
$step->processOperator($operator);
}
Expand Down
20 changes: 20 additions & 0 deletions lib/private/Files/Search/QueryOptimizer/QueryOptimizerStep.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@
use OCP\Files\Search\ISearchOperator;

class QueryOptimizerStep {
/**
* Allow optimizer steps to inspect the entire query before starting processing
*
* @param ISearchOperator $operator
* @return void
*/
public function inspectOperator(ISearchOperator $operator): void {
if ($operator instanceof ISearchBinaryOperator) {
foreach ($operator->getArguments() as $argument) {
$this->inspectOperator($argument);
}
}
}

/**
* Allow optimizer steps to modify query operators
*
* @param ISearchOperator $operator
* @return void
*/
public function processOperator(ISearchOperator &$operator) {
if ($operator instanceof ISearchBinaryOperator) {
foreach ($operator->getArguments() as $argument) {
Expand Down