From 747ed3900ddad5ea9bea803bb0761c225bdede3e Mon Sep 17 00:00:00 2001
From: Robin Appelman <robin@icewind.nl>
Date: Wed, 11 Jan 2023 16:38:41 +0100
Subject: [PATCH] extend path-prefix optimizer to remove all cases of
 path_hash= when encountering a path prefix filter

Signed-off-by: Robin Appelman <robin@icewind.nl>
---
 .../QueryOptimizer/PathPrefixOptimizer.php    | 48 ++++++++++++++-----
 .../Search/QueryOptimizer/QueryOptimizer.php  |  3 ++
 .../QueryOptimizer/QueryOptimizerStep.php     | 20 ++++++++
 3 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/lib/private/Files/Search/QueryOptimizer/PathPrefixOptimizer.php b/lib/private/Files/Search/QueryOptimizer/PathPrefixOptimizer.php
index 62182303ffd..0caa9b12a02 100644
--- a/lib/private/Files/Search/QueryOptimizer/PathPrefixOptimizer.php
+++ b/lib/private/Files/Search/QueryOptimizer/PathPrefixOptimizer.php
@@ -29,27 +29,49 @@ use OCP\Files\Search\ISearchComparison;
 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()) . '/%'
+		);
 	}
 }
diff --git a/lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php b/lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php
index 1635e50335a..160b27b7f11 100644
--- a/lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php
+++ b/lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php
@@ -38,6 +38,9 @@ class QueryOptimizer {
 	}
 
 	public function processOperator(ISearchOperator $operator) {
+		foreach ($this->steps as $step) {
+			$step->inspectOperator($operator);
+		}
 		foreach ($this->steps as $step) {
 			$step->processOperator($operator);
 		}
diff --git a/lib/private/Files/Search/QueryOptimizer/QueryOptimizerStep.php b/lib/private/Files/Search/QueryOptimizer/QueryOptimizerStep.php
index 4f683899723..b16898e9087 100644
--- a/lib/private/Files/Search/QueryOptimizer/QueryOptimizerStep.php
+++ b/lib/private/Files/Search/QueryOptimizer/QueryOptimizerStep.php
@@ -27,6 +27,26 @@ use OCP\Files\Search\ISearchBinaryOperator;
 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) {
-- 
2.39.5