diff options
3 files changed, 164 insertions, 39 deletions
diff --git a/lib/private/Files/Search/QueryOptimizer/MergeDistributiveOperations.php b/lib/private/Files/Search/QueryOptimizer/MergeDistributiveOperations.php index 922b0ccb17c..7986df82adc 100644 --- a/lib/private/Files/Search/QueryOptimizer/MergeDistributiveOperations.php +++ b/lib/private/Files/Search/QueryOptimizer/MergeDistributiveOperations.php @@ -18,21 +18,19 @@ use OCP\Files\Search\ISearchOperator; */ class MergeDistributiveOperations extends ReplacingOptimizerStep { public function processOperator(ISearchOperator &$operator): bool { - if ( - $operator instanceof SearchBinaryOperator && - $this->isAllSameBinaryOperation($operator->getArguments()) - ) { + if ($operator instanceof SearchBinaryOperator) { // either 'AND' or 'OR' $topLevelType = $operator->getType(); // split the arguments into groups that share a first argument - // (we already know that all arguments are binary operators with at least 1 child) $groups = $this->groupBinaryOperatorsByChild($operator->getArguments(), 0); $outerOperations = array_map(function (array $operators) use ($topLevelType) { // no common operations, no need to change anything if (count($operators) === 1) { return $operators[0]; } + + // for groups with size >1 we know they are binary operators with at least 1 child /** @var ISearchBinaryOperator $firstArgument */ $firstArgument = $operators[0]; @@ -73,45 +71,24 @@ class MergeDistributiveOperations extends ReplacingOptimizerStep { } /** - * Check that a list of operators is all the same type of (non-empty) binary operators - * - * @param ISearchOperator[] $operators - * @return bool - * @psalm-assert-if-true SearchBinaryOperator[] $operators - */ - private function isAllSameBinaryOperation(array $operators): bool { - $operation = null; - foreach ($operators as $operator) { - if (!$operator instanceof SearchBinaryOperator) { - return false; - } - if (!$operator->getArguments()) { - return false; - } - if ($operation === null) { - $operation = $operator->getType(); - } else { - if ($operation !== $operator->getType()) { - return false; - } - } - } - return true; - } - - /** * Group a list of binary search operators that have a common argument * - * @param SearchBinaryOperator[] $operators - * @return SearchBinaryOperator[][] + * Non-binary operators, or empty binary operators will each get their own 1-sized group + * + * @param ISearchOperator[] $operators + * @return ISearchOperator[][] */ private function groupBinaryOperatorsByChild(array $operators, int $index = 0): array { $result = []; foreach ($operators as $operator) { - /** @var SearchBinaryOperator|SearchComparison $child */ - $child = $operator->getArguments()[$index]; - $childKey = (string) $child; - $result[$childKey][] = $operator; + if ($operator instanceof ISearchBinaryOperator && count($operator->getArguments()) > 0) { + /** @var SearchBinaryOperator|SearchComparison $child */ + $child = $operator->getArguments()[$index]; + $childKey = (string)$child; + $result[$childKey][] = $operator; + } else { + $result[] = [$operator]; + } } return array_values($result); } diff --git a/lib/private/Files/Search/QueryOptimizer/ReplacingOptimizerStep.php b/lib/private/Files/Search/QueryOptimizer/ReplacingOptimizerStep.php index 546061522bc..473f8a87151 100644 --- a/lib/private/Files/Search/QueryOptimizer/ReplacingOptimizerStep.php +++ b/lib/private/Files/Search/QueryOptimizer/ReplacingOptimizerStep.php @@ -20,7 +20,9 @@ class ReplacingOptimizerStep extends QueryOptimizerStep { $modified = false; $arguments = $operator->getArguments(); foreach ($arguments as &$argument) { - $modified = $modified || $this->processOperator($argument); + if ($this->processOperator($argument)) { + $modified = true; + } } if ($modified) { $operator->setArguments($arguments); diff --git a/tests/lib/Files/Search/QueryOptimizer/CombinedTests.php b/tests/lib/Files/Search/QueryOptimizer/CombinedTests.php index c1d0428176d..c5abd5603da 100644 --- a/tests/lib/Files/Search/QueryOptimizer/CombinedTests.php +++ b/tests/lib/Files/Search/QueryOptimizer/CombinedTests.php @@ -42,4 +42,150 @@ class CombinedTests extends TestCase { $this->assertEquals('(storage eq 1 and path in ["foo","bar","asd"])', $operator->__toString()); } + + public function testComplexSearchPattern1() { + $operator = new SearchBinaryOperator( + ISearchBinaryOperator::OPERATOR_OR, + [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 1), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 2), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "201"), + new SearchComparison(ISearchComparison::COMPARE_LIKE, "path", "201/%"), + ]), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "301"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 4), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "401"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "302"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 4), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "402"), + ]), + ] + ); + $this->assertEquals('((storage eq 1) or (storage eq 2 and (path eq "201" or path like "201\/%")) or (storage eq 3 and path eq "301") or (storage eq 4 and path eq "401") or (storage eq 3 and path eq "302") or (storage eq 4 and path eq "402"))', $operator->__toString()); + + $this->optimizer->processOperator($operator); + + $this->assertEquals('(storage eq 1 or (storage eq 2 and (path eq "201" or path like "201\/%")) or (storage eq 3 and path in ["301","302"]) or (storage eq 4 and path in ["401","402"]))', $operator->__toString()); + } + + public function testComplexSearchPattern2() { + $operator = new SearchBinaryOperator( + ISearchBinaryOperator::OPERATOR_OR, + [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 1), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 2), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "201"), + new SearchComparison(ISearchComparison::COMPARE_LIKE, "path", "201/%"), + ]), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "301"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 4), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "401"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "302"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 4), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "402"), + ]), + ] + ); + $this->assertEquals('(storage eq 1 or (storage eq 2 and (path eq "201" or path like "201\/%")) or (storage eq 3 and path eq "301") or (storage eq 4 and path eq "401") or (storage eq 3 and path eq "302") or (storage eq 4 and path eq "402"))', $operator->__toString()); + + $this->optimizer->processOperator($operator); + + $this->assertEquals('(storage eq 1 or (storage eq 2 and (path eq "201" or path like "201\/%")) or (storage eq 3 and path in ["301","302"]) or (storage eq 4 and path in ["401","402"]))', $operator->__toString()); + } + + public function testApplySearchConstraints1() { + $operator = new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "mimetype", "image/png"), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "mimetype", "image/jpeg"), + ]), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 1), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "files"), + new SearchComparison(ISearchComparison::COMPARE_LIKE, "path", "files/%"), + ]), + ]), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 2), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "files/301"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "files/302"), + ]), + ]), + ]); + $this->assertEquals('(((mimetype eq "image\/png" or mimetype eq "image\/jpeg")) and ((storage eq 1 and (path eq "files" or path like "files\/%")) or storage eq 2 or (storage eq 3 and path eq "files\/301") or (storage eq 3 and path eq "files\/302")))', $operator->__toString()); + + $this->optimizer->processOperator($operator); + + $this->assertEquals('(mimetype in ["image\/png","image\/jpeg"] and ((storage eq 1 and (path eq "files" or path like "files\/%")) or storage eq 2 or (storage eq 3 and path in ["files\/301","files\/302"])))', $operator->__toString()); + } + + public function testApplySearchConstraints2() { + $operator = new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "mimetype", "image/png"), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "mimetype", "image/jpeg"), + ]), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 1), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "files"), + new SearchComparison(ISearchComparison::COMPARE_LIKE, "path", "files/%"), + ]), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 2), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "files/301"), + ]), + new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, [ + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "storage", 3), + new SearchComparison(ISearchComparison::COMPARE_EQUAL, "path", "files/302"), + ]), + ]), + ]); + $this->assertEquals('(((mimetype eq "image\/png" or mimetype eq "image\/jpeg")) and ((storage eq 1 and (path eq "files" or path like "files\/%")) or (storage eq 2) or (storage eq 3 and path eq "files\/301") or (storage eq 3 and path eq "files\/302")))', $operator->__toString()); + + $this->optimizer->processOperator($operator); + + $this->assertEquals('(mimetype in ["image\/png","image\/jpeg"] and ((storage eq 1 and (path eq "files" or path like "files\/%")) or storage eq 2 or (storage eq 3 and path in ["files\/301","files\/302"])))', $operator->__toString()); + } } |