fix(core): handle parameter limits in files metadata deletion - #62331
fix(core): handle parameter limits in files metadata deletion#62331gitedmond wants to merge 1 commit into
Conversation
a56d71c to
48551eb
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
come-nc
left a comment
There was a problem hiding this comment.
Hello,
Please switch back to using the const MAX_IN_PARAMETERS, we do not want to have a magic number 500 in there.
Nice catch on the wrong var being used.
Regarding the transaction I lack knowledge to judge in it makes sense here. Is a partial metadata deletion an issue?
You're right: the transaction is not needed for this fix, and it would not make metadata and index deletion atomic because those happen in separate calls. I've removed it. The PR now only binds each existing |
6a4e471 to
071b333
Compare
Bind each existing MAX_IN_PARAMETERS chunk in dropMetadataForFiles instead of passing the full file ID array to every query. Add a regression test that verifies the exact chunks sent to the query builder. Resolves: nextcloud#62325 Signed-off-by: Edmond <edmnd@users.noreply.github.com>
071b333 to
12ff385
Compare
Summary
dropMetadataForFiles()was already intended to split file IDs into chunks usingIQueryBuilder::MAX_IN_PARAMETERS, but the DELETE query mistakenly bound the original$fileIdsarray instead of the current$chunk. As a result, every query still contained the full ID list and could exceed database parameter limits.This PR passes
$chunkto each query and adds regression coverage that verifies the exact chunks sent to the query builder.Testing
Added a regression test using more than two parameter-limit batches. It verifies that every ID is bound exactly once, in order, and no query receives more than
MAX_IN_PARAMETERSIDs.AI (if applicable)