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

fix(Files): use prepare statement for path_hash in propagator #49469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Nov 25, 2024

Summary

The other day I had to analyze SQL queries. And in the log I found entries like:

UPDATE "oc_filecache" SET "mtime" = GREATEST("mtime", $1), "etag" = $2 
WHERE ("storage" = $3) 
    AND ("path_hash" IN ('0123456789abcdef0123456789abcdef', '123456789abcdef0123456789abcdef0', '23456789abcdef0123456789abcdef01', '3456789abcdef0123456789abcdef012', '456789abcdef0123456789abcdef0123', '56789abcdef0123456789abcdef01234', '6789abcdef0123456789abcdef012345', '789abcdef0123456789abcdef0123456')) 

As you see, some parameters where having placeholders (originating from use of prepared statements) while the IN list had not. So it was not possible to simply sum up this query, because it contained differing values.

Checklist

@blizzz blizzz added 3. to review Waiting for reviews technical debt feature: database Database related DB labels Nov 25, 2024
@blizzz blizzz added this to the Nextcloud 31 milestone Nov 25, 2024
@@ -80,7 +80,7 @@ public function propagateChange($internalPath, $time, $sizeDifference = 0) {
$builder->update('filecache')
->set('mtime', $builder->func()->greatest('mtime', $builder->createNamedParameter((int)$time, IQueryBuilder::PARAM_INT)))
->where($builder->expr()->eq('storage', $builder->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
->andWhere($builder->expr()->in('path_hash', $hashParams));
->andWhere($builder->expr()->in('path_hash', $builder->createNamedParameter($hashParams, IQueryBuilder::PARAM_STR_ARRAY)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should then not return $builder->expr()->literal($hash); above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews feature: database Database related DB technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants