Skip to content

Commit

Permalink
optimize query pattern used by storage filter
Browse files Browse the repository at this point in the history
Signed-off-by: Robin Appelman <robin@icewind.nl>
  • Loading branch information
icewind1991 committed Sep 21, 2023
1 parent 09794b6 commit 3b637f8
Show file tree
Hide file tree
Showing 13 changed files with 603 additions and 40 deletions.
112 changes: 81 additions & 31 deletions lib/private/Files/Cache/SearchBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class SearchBuilder {
ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'gte',
ISearchComparison::COMPARE_LESS_THAN => 'lt',
ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'lte',
ISearchComparison::COMPARE_IN => 'in',
];

protected static $searchOperatorNegativeMap = [
Expand All @@ -55,6 +56,31 @@ class SearchBuilder {
ISearchComparison::COMPARE_GREATER_THAN_EQUAL => 'lt',
ISearchComparison::COMPARE_LESS_THAN => 'gte',
ISearchComparison::COMPARE_LESS_THAN_EQUAL => 'gt',
ISearchComparison::COMPARE_IN => 'notIn',
];

protected static $fieldTypes = [
'mimetype' => 'string',
'mtime' => 'integer',
'name' => 'string',
'path' => 'string',
'size' => 'integer',
'tagname' => 'string',
'systemtag' => 'string',
'favorite' => 'boolean',
'fileid' => 'integer',
'storage' => 'integer',
];

protected static $paramTypeMap = [
'string' => IQueryBuilder::PARAM_STR,
'integer' => IQueryBuilder::PARAM_INT,
'boolean' => IQueryBuilder::PARAM_INT,
];
protected static $paramArrayTypeMap = [
'string' => IQueryBuilder::PARAM_STR_ARRAY,
'integer' => IQueryBuilder::PARAM_INT_ARRAY,
'boolean' => IQueryBuilder::PARAM_INT_ARRAY,
];

public const TAG_FAVORITE = '_$!<Favorite>!$_';
Expand Down Expand Up @@ -108,7 +134,7 @@ public function searchOperatorToDBExpr(IQueryBuilder $builder, ISearchOperator $
} else {
throw new \InvalidArgumentException('Binary operators inside "not" is not supported');
}
// no break
// no break
case ISearchBinaryOperator::OPERATOR_AND:
return call_user_func_array([$expr, 'andX'], $this->searchOperatorArrayToDBExprArray($builder, $operator->getArguments()));
case ISearchBinaryOperator::OPERATOR_OR:
Expand All @@ -129,21 +155,46 @@ private function searchComparisonToDBExpr(IQueryBuilder $builder, ISearchCompari
[$field, $value, $type] = $this->getOperatorFieldAndValue($comparison);
if (isset($operatorMap[$type])) {
$queryOperator = $operatorMap[$type];
return $builder->expr()->$queryOperator($field, $this->getParameterForValue($builder, $value));
return $builder->expr()->$queryOperator($field, $this->getParameterForValue($builder, $value, self::$fieldTypes[$comparison->getField()]));
} else {
throw new \InvalidArgumentException('Invalid operator type: ' . $comparison->getType());
}
}

private function getOperatorFieldAndValue(ISearchComparison $operator) {
/**
* @param ISearchComparison $operator
* @return list{string, string|integer|\DateTime|(\DateTime|int|string)[], string}
*/
private function getOperatorFieldAndValue(ISearchComparison $operator): array {
$field = $operator->getField();
$value = $operator->getValue();
$type = $operator->getType();
$pathEqHash = $operator->getQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, true);
return $this->getOperatorFieldAndValueInner($field, $value, $type, $pathEqHash);
}

/**
* @param string $field
* @param string|integer|\DateTime|(\DateTime|int|string)[] $value
* @param string $type
* @return list{string, string|integer|\DateTime|(\DateTime|int|string)[], string}

Check failure on line 180 in lib/private/Files/Cache/SearchBuilder.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

MoreSpecificReturnType

lib/private/Files/Cache/SearchBuilder.php:180:13: MoreSpecificReturnType: The declared return type 'list{string, DateTime|array<array-key, DateTime|int|string>|int|string, string}' for OC\Files\Cache\SearchBuilder::getOperatorFieldAndValueInner is more specific than the inferred return type 'list{string, DateTime|array<array-key, DateTime|array<array-key, DateTime|int|string>|int|string>|int|string, string}' (see https://psalm.dev/070)

Check failure

Code scanning / Psalm

MoreSpecificReturnType Error

The declared return type 'list{string, DateTime|array<array-key, DateTime|int|string>|int|string, string}' for OC\Files\Cache\SearchBuilder::getOperatorFieldAndValueInner is more specific than the inferred return type 'list{string, DateTime|array<array-key, DateTime|array<array-key, DateTime|int|string>|int|string>|int|string, string}'
*/
private function getOperatorFieldAndValueInner(string $field, mixed $value, string $type, bool $pathEqHash): array {
if ($type === ISearchComparison::COMPARE_IN) {
$resultField = $field;
$values = [];
foreach($value as $arrayValue) {
$result = $this->getOperatorFieldAndValueInner($field, $arrayValue, ISearchComparison::COMPARE_EQUAL, $pathEqHash);
$resultField = $result[0];
$values[] = $result[1];
}
return [$resultField, $values, ISearchComparison::COMPARE_IN];

Check failure on line 191 in lib/private/Files/Cache/SearchBuilder.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidReturnStatement

lib/private/Files/Cache/SearchBuilder.php:191:11: InvalidReturnStatement: The inferred type 'list{string, list<DateTime|array<array-key, DateTime|int|string>|int|string>, 'in'}' does not match the declared return type 'list{string, DateTime|array<array-key, DateTime|int|string>|int|string, string}' for OC\Files\Cache\SearchBuilder::getOperatorFieldAndValueInner (see https://psalm.dev/128)

Check failure

Code scanning / Psalm

InvalidReturnStatement Error

The inferred type 'list{string, list<DateTime|array<array-key, DateTime|int|string>|int|string>, 'in'}' does not match the declared return type 'list{string, DateTime|array<array-key, DateTime|int|string>|int|string, string}' for OC\Files\Cache\SearchBuilder::getOperatorFieldAndValueInner
}
if ($field === 'mimetype') {
$value = (string)$value;
if ($operator->getType() === ISearchComparison::COMPARE_EQUAL) {
if ($type === ISearchComparison::COMPARE_EQUAL) {
$value = (int)$this->mimetypeLoader->getId($value);
} elseif ($operator->getType() === ISearchComparison::COMPARE_LIKE) {
} elseif ($type === ISearchComparison::COMPARE_LIKE) {
// transform "mimetype='foo/%'" to "mimepart='foo'"
if (preg_match('|(.+)/%|', $value, $matches)) {
$field = 'mimepart';
Expand All @@ -168,59 +219,58 @@ private function getOperatorFieldAndValue(ISearchComparison $operator) {
$field = 'systemtag.name';
} elseif ($field === 'fileid') {
$field = 'file.fileid';
} elseif ($field === 'path' && $type === ISearchComparison::COMPARE_EQUAL && $operator->getQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, true)) {
} elseif ($field === 'path' && $type === ISearchComparison::COMPARE_EQUAL && $pathEqHash) {
$field = 'path_hash';
$value = md5((string)$value);
}
return [$field, $value, $type];
}

private function validateComparison(ISearchComparison $operator) {
$types = [
'mimetype' => 'string',
'mtime' => 'integer',
'name' => 'string',
'path' => 'string',
'size' => 'integer',
'tagname' => 'string',
'systemtag' => 'string',
'favorite' => 'boolean',
'fileid' => 'integer',
'storage' => 'integer',
];
$comparisons = [
'mimetype' => ['eq', 'like'],
'mimetype' => ['eq', 'like', 'in'],
'mtime' => ['eq', 'gt', 'lt', 'gte', 'lte'],
'name' => ['eq', 'like', 'clike'],
'path' => ['eq', 'like', 'clike'],
'name' => ['eq', 'like', 'clike', 'in'],
'path' => ['eq', 'like', 'clike', 'in'],
'size' => ['eq', 'gt', 'lt', 'gte', 'lte'],
'tagname' => ['eq', 'like'],
'systemtag' => ['eq', 'like'],
'favorite' => ['eq'],
'fileid' => ['eq'],
'storage' => ['eq'],
'fileid' => ['eq', 'in'],
'storage' => ['eq', 'in'],
];

if (!isset($types[$operator->getField()])) {
if (!isset(self::$fieldTypes[$operator->getField()])) {
throw new \InvalidArgumentException('Unsupported comparison field ' . $operator->getField());
}
$type = $types[$operator->getField()];
if (gettype($operator->getValue()) !== $type) {
throw new \InvalidArgumentException('Invalid type for field ' . $operator->getField());
$type = self::$fieldTypes[$operator->getField()];
if ($operator->getType() === ISearchComparison::COMPARE_IN) {
if (!is_array($operator->getValue())) {
throw new \InvalidArgumentException('Invalid type for field ' . $operator->getField());
}
foreach ($operator->getValue() as $arrayValue) {
if (gettype($arrayValue) !== $type) {
throw new \InvalidArgumentException('Invalid type in array for field ' . $operator->getField());
}
}
} else {
if (gettype($operator->getValue()) !== $type) {
throw new \InvalidArgumentException('Invalid type for field ' . $operator->getField());
}
}
if (!in_array($operator->getType(), $comparisons[$operator->getField()])) {
throw new \InvalidArgumentException('Unsupported comparison for field ' . $operator->getField() . ': ' . $operator->getType());
}
}

private function getParameterForValue(IQueryBuilder $builder, $value) {
private function getParameterForValue(IQueryBuilder $builder, $value, string $paramType) {
if ($value instanceof \DateTime) {
$value = $value->getTimestamp();
}
if (is_numeric($value)) {
$type = IQueryBuilder::PARAM_INT;
if (is_array($value)) {
$type = self::$paramArrayTypeMap[$paramType];
} else {
$type = IQueryBuilder::PARAM_STR;
$type = self::$paramTypeMap[$paramType];
}
return $builder->createNamedParameter($value, $type);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace OC\Files\Search\QueryOptimizer;

use OCP\Files\Search\ISearchBinaryOperator;
use OCP\Files\Search\ISearchOperator;

/**
* replace single argument AND and OR operations with their single argument
*/
class FlattenSingleArgumentBinaryOperation extends ReplacingOptimizerStep {
public function processOperator(ISearchOperator &$operator): bool {
parent::processOperator($operator);
if (
$operator instanceof ISearchBinaryOperator &&
count($operator->getArguments()) === 1 &&
(
$operator->getType() === ISearchBinaryOperator::OPERATOR_OR ||
$operator->getType() === ISearchBinaryOperator::OPERATOR_AND
)
) {
$operator = $operator->getArguments()[0];
return true;
}
return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

namespace OC\Files\Search\QueryOptimizer;

use OC\Files\Search\SearchBinaryOperator;
use OCP\Files\Search\ISearchBinaryOperator;
use OCP\Files\Search\ISearchOperator;

/**
* Attempt to transform
*
* (A AND B) OR (A AND C) into A AND (B OR C)
*/
class MergeDistributiveOperations extends ReplacingOptimizerStep {
public function processOperator(ISearchOperator &$operator): bool {
if (
$operator instanceof SearchBinaryOperator &&
$this->isAllSameBinaryOperation($operator->getArguments())
) {
$topLevelType = $operator->getType();

$groups = $this->groupBinaryOperatorsByChild($operator->getArguments(), 0);
$outerOperations = array_map(function(array $operators) use ($topLevelType) {
/** @var ISearchBinaryOperator $firstArgument */
$firstArgument = $operators[0];
$outerType = $firstArgument->getType();
$extractedLeftHand = $firstArgument->getArguments()[0];

$rightHandArguments = array_map(function(ISearchOperator $inner) {
/** @var ISearchBinaryOperator $inner */
$arguments = $inner->getArguments();
array_shift($arguments);
if (count($arguments) === 1){
return $arguments[0];
}
return new SearchBinaryOperator($inner->getType(), $arguments);
}, $operators);
$extractedRightHand = new SearchBinaryOperator($topLevelType, $rightHandArguments);
return new SearchBinaryOperator(
$outerType,
[$extractedLeftHand, $extractedRightHand]
);
}, $groups);
$operator = new SearchBinaryOperator($topLevelType, $outerOperations);
parent::processOperator($operator);
return true;
}
return parent::processOperator($operator);
}

/**
* 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[][]
*/
private function groupBinaryOperatorsByChild(array $operators, int $index): array {
$result = [];
foreach($operators as $operator) {
$childKey = (string) $operator->getArguments()[0];

Check failure on line 87 in lib/private/Files/Search/QueryOptimizer/MergeDistributiveOperations.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

InvalidCast

lib/private/Files/Search/QueryOptimizer/MergeDistributiveOperations.php:87:25: InvalidCast: OCP\Files\Search\ISearchOperator cannot be cast to string (see https://psalm.dev/103)

Check failure

Code scanning / Psalm

InvalidCast Error

OCP\Files\Search\ISearchOperator cannot be cast to string
$result[$childKey][] = $operator;
}
return array_values($result);
}
}
67 changes: 67 additions & 0 deletions lib/private/Files/Search/QueryOptimizer/OrEqualsToIn.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

namespace OC\Files\Search\QueryOptimizer;

use OC\Files\Search\SearchBinaryOperator;
use OC\Files\Search\SearchComparison;
use OCP\Files\Search\ISearchBinaryOperator;
use OCP\Files\Search\ISearchComparison;
use OCP\Files\Search\ISearchOperator;

/**
* transform (field == A OR field == B ...) into field IN (A, B, ...)
*/
class OrEqualsToIn extends ReplacingOptimizerStep {
public function processOperator(ISearchOperator &$operator): bool {
if (
$operator instanceof ISearchBinaryOperator &&
$operator->getType() == ISearchBinaryOperator::OPERATOR_OR
) {
$groups = $this->groupEqualsComparisonsByField($operator->getArguments());
$newParts = array_map(function(array $group) {
if (count($group) > 1) {
// because of the logic from `groupEqualsComparisonsByField` we now that group is all comparisons on the same field
/** @var ISearchComparison[] $group */
$field = $group[0]->getField();
$values = array_map(function(ISearchComparison $comparison) {
/** @var string|integer|\DateTime $value */
$value = $comparison->getValue();
return $value;
}, $group);
$in = new SearchComparison(ISearchComparison::COMPARE_IN, $field, $values);
$in->setQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, $group[0]->getQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, true));
return $in;
} else {
return $group[0];
}
}, $groups);
if (count($newParts) === 1) {
$operator = $newParts[0];
} else {
$operator = new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, $newParts);
}
return true;
}
parent::processOperator($operator);
return false;
}

/**
* Non-equals operators are put in a separate group for each
*
* @param ISearchOperator[] $operators
* @return ISearchOperator[][]
*/
private function groupEqualsComparisonsByField(array $operators): array {
$result = [];
foreach ($operators as $operator) {
if ($operator instanceof ISearchComparison && $operator->getType() === ISearchComparison::COMPARE_EQUAL) {
$key = $operator->getField() . $operator->getQueryHint(ISearchComparison::HINT_PATH_EQ_HASH, true);
$result[$key][] = $operator;
} else {
$result[] = [$operator];
}
}
return array_values($result);
}
}
13 changes: 10 additions & 3 deletions lib/private/Files/Search/QueryOptimizer/QueryOptimizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,21 @@ class QueryOptimizer {
private $steps = [];

public function __construct(
PathPrefixOptimizer $pathPrefixOptimizer
PathPrefixOptimizer $pathPrefixOptimizer,
MergeDistributiveOperations $mergeDistributiveOperations,
FlattenSingleArgumentBinaryOperation $flattenSingleArgumentBinaryOperation,
OrEqualsToIn $orEqualsToIn
) {
// note that the order here is relevant
$this->steps = [
$pathPrefixOptimizer
$pathPrefixOptimizer,
$mergeDistributiveOperations,
$flattenSingleArgumentBinaryOperation,
$orEqualsToIn,
];
}

public function processOperator(ISearchOperator $operator) {
public function processOperator(ISearchOperator &$operator) {
foreach ($this->steps as $step) {
$step->inspectOperator($operator);
}
Expand Down
Loading

0 comments on commit 3b637f8

Please sign in to comment.