Skip to content

Allow insertIgnoreConflict to work with sharding and use it for filecache.#59732

Open
tcitworld wants to merge 3 commits intomasterfrom
bug/19494/insert-ignore-conflict-for-filecache-extended-rework
Open

Allow insertIgnoreConflict to work with sharding and use it for filecache.#59732
tcitworld wants to merge 3 commits intomasterfrom
bug/19494/insert-ignore-conflict-for-filecache-extended-rework

Conversation

@tcitworld
Copy link
Copy Markdown
Member

@tcitworld tcitworld commented Apr 20, 2026 •
edited
Loading

Summary

Replaces #48564

As we added hintShardKey to the query since #48564 was opened, it can't be resolved directly through use of insertIgnoreConflict anymore. To set hintShardKey anyway, I've added an extra optional 3rd parameter to do that.

As the insertIgnoreConflict methods in the database adapters use the connection directly to alter the produced SQL, bypassing the QueryBuilder and therefore the ShardedQueryBuilder when sharding is activated, I added a ignoreConflictsOnInsert method to IQueryBuilder we can use to tell the QB to apply a callable provided by the DB adapters to alter the produced SQL in this specific case.

In OC\Files\Cache\Cache, I'm bypassing the CacheQueryBuilder and using insertIgnoreConflict directly on the connection. It's possible this is very wrong, please advise.

TODO

  • Verify that bypassing CacheQueryBuilder in this case is fine
  • Check it actually works
  • See what kind of tests can be added for that

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@tcitworld tcitworld self-assigned this Apr 20, 2026
@tcitworld tcitworld added 2. developing Work in progress feature: files feature: database Database related DB labels Apr 20, 2026
@tcitworld tcitworld mentioned this pull request Apr 20, 2026
Open
@tcitworld tcitworld force-pushed the bug/19494/insert-ignore-conflict-for-filecache-extended-rework branch from 86ac08b to d821eb2 Compare April 20, 2026 13:52
nickvergessen
nickvergessen requested changes Apr 21, 2026
View reviewed changes
Comment thread lib/public/IDBConnection.php Outdated
Comment thread lib/public/IDBConnection.php Outdated
@tcitworld tcitworld force-pushed the bug/19494/insert-ignore-conflict-for-filecache-extended-rework branch 2 times, most recently from 7577060 to 0e35b8c Compare April 22, 2026 16:10
@tcitworld tcitworld requested a review from nickvergessen April 22, 2026 16:11
@tcitworld tcitworld force-pushed the bug/19494/insert-ignore-conflict-for-filecache-extended-rework branch 2 times, most recently from ef0ec48 to 6481a5f Compare April 22, 2026 19:40
@tcitworld tcitworld force-pushed the bug/19494/insert-ignore-conflict-for-filecache-extended-rework branch from 6481a5f to 6cf0d7a Compare April 23, 2026 16:59
@tcitworld tcitworld marked this pull request as ready for review April 23, 2026 16:59
@tcitworld tcitworld requested a review from a team as a code owner April 23, 2026 16:59
@tcitworld tcitworld requested review from Altahrim, CarlSchwan and salmart-dev and removed request for a team April 23, 2026 16:59
@nickvergessen
Copy link
Copy Markdown
Member

nickvergessen commented Apr 24, 2026

Sharding tests are failing 🙈

@tcitworld tcitworld force-pushed the bug/19494/insert-ignore-conflict-for-filecache-extended-rework branch from 6cf0d7a to e3e9529 Compare April 26, 2026 15:51
@tcitworld
feat(db): add optional hintShardKey parameter to insertIgnoreConflict
f238f60 
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@tcitworld
feat(db): introduce a ignoreConflictsOnInsert method on the QueryBuilder
ab3d4c6 
Since the DB adapters insertIgnoreConflict methods are using the conn directly to alter the produced
SQL, bypassing the ShardedQueryBuilder logic, hinting for sharded keys didn't work. We need to
expose to the QueryBuilder the information that we want to ignore conflicts, and how to handle it.

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@tcitworld
feat(cache): use the insertIgnoreConflict connexion method so that co…
fe78872 
…nflicts are properly handled

In the FileCache, use insertIgnoreConflict to avoid conflict error messages on PostgreSQL. Use the
new optional hintShardKey parameter

Closes #19494

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
@tcitworld tcitworld force-pushed the bug/19494/insert-ignore-conflict-for-filecache-extended-rework branch from e3e9529 to fe78872 Compare April 26, 2026 15:52
@tcitworld
Copy link
Copy Markdown
Member Author

tcitworld commented Apr 26, 2026

Sharding tests are failing 🙈

The adapters were calling the connection directly without using the QueryBuilder, so the sharding logic couldn't apply through ShardedQueryBuilder. I introduced a ignoreConflictsOnInsert method to IQueryBuilder so that we can alter the SQL produced to add the ignore instructions.

This focuses on the ignoring conflict cases, but it could be changed a little to have something like a sqlPostProcessing callback that would alter the SQL produced depending on options (where ignoring conflicts would only be one of them).

Obviously all of this would be easier if Doctrine DBAL supported ignoring conflicts itself, but 🤷

@tcitworld tcitworld added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 26, 2026
@tcitworld tcitworld changed the title Add optional callable argument to insertIgnoreConflict so that hintShardKey can be set too Allow insertIgnoreConflict to work with sharding and use it for filecache. Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@kesselb kesselb Awaiting requested review from kesselb

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@Altahrim Altahrim Awaiting requested review from Altahrim Altahrim is a code owner automatically assigned from nextcloud/server-backend

@salmart-dev salmart-dev Awaiting requested review from salmart-dev salmart-dev is a code owner automatically assigned from nextcloud/server-backend

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan CarlSchwan is a code owner automatically assigned from nextcloud/server-backend

Requested changes must be addressed to merge this pull request.

Assignees

@tcitworld tcitworld

Labels

3. to review Waiting for reviews feature: database Database related DB feature: files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PostgreSQL duplicate key value violates unique constraint

2 participants

@tcitworld @nickvergessen