Skip to content

refactor(dbal): move from deprecated to modern calls - #58891

Merged
CarlSchwan merged 2 commits into
masterfrom
executeUpdateRemove
Jul 16, 2026
Merged

refactor(dbal): move from deprecated to modern calls#58891
CarlSchwan merged 2 commits into
masterfrom
executeUpdateRemove

Conversation

@solracsf

@solracsf solracsf commented Mar 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Give Doctrine DBAL some love 💘 by moving away from deprecated calls and a few RAW queries.

Checklist

AI (if applicable)

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

@solracsf solracsf added this to the Nextcloud 34 milestone Mar 12, 2026
@solracsf solracsf added the feature: database Database related DB label Mar 12, 2026
@solracsf solracsf changed the title chore(dbal): move to executeStatement chore(dbal): WiP Mar 12, 2026
@solracsf solracsf added 2. developing Work in progress ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Mar 12, 2026
@solracsf
solracsf force-pushed the executeUpdateRemove branch from e9754ab to 2a6989f Compare March 13, 2026 06:25
@solracsf solracsf changed the title chore(dbal): WiP chore(dbal): refactor to modern calls Mar 13, 2026
@solracsf solracsf added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 13, 2026
@solracsf solracsf changed the title chore(dbal): refactor to modern calls refactor(dbal): move from deprecated to modern calls Mar 13, 2026
@solracsf
solracsf marked this pull request as ready for review March 13, 2026 07:16
@solracsf
solracsf requested review from ArtificialOwl, leftybournes, nfebe and sorbaugh and removed request for a team March 13, 2026 07:16
@solracsf
solracsf force-pushed the executeUpdateRemove branch 2 times, most recently from b8d7a73 to 21416b8 Compare March 13, 2026 08:20
SebastianKrupinski
SebastianKrupinski approved these changes Mar 15, 2026
View reviewed changes

@SebastianKrupinski SebastianKrupinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CalDav looks fine

miaulalala
miaulalala reviewed Mar 16, 2026
View reviewed changes
Comment thread lib/private/DB/ArrayResult.php Outdated
miaulalala
miaulalala previously requested changes Mar 17, 2026
View reviewed changes

@miaulalala miaulalala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as per my other comment

@nextcloud-bot nextcloud-bot mentioned this pull request May 27, 2026
Merged
@nextcloud-bot nextcloud-bot mentioned this pull request Jun 4, 2026
Closed
@skjnldsv skjnldsv mentioned this pull request Jun 4, 2026
Merged
This was referenced Jun 5, 2026
Merged
Merged
@susnux susnux added the community pull requests from community label Jun 9, 2026
@susnux susnux modified the milestones: Nextcloud 34.0.1, Nextcloud 35 Jun 9, 2026
@CarlSchwan
CarlSchwan force-pushed the executeUpdateRemove branch from 21416b8 to c80ae66 Compare June 11, 2026 10:02
@CarlSchwan
CarlSchwan enabled auto-merge June 11, 2026 10:02
@CarlSchwan CarlSchwan self-assigned this Jun 11, 2026
@miaulalala
miaulalala force-pushed the executeUpdateRemove branch from c80ae66 to 5dda39a Compare June 24, 2026 08:35
@susnux
susnux force-pushed the executeUpdateRemove branch from 5dda39a to e5ce498 Compare July 14, 2026 21:23
SebastianKrupinski
SebastianKrupinski approved these changes Jul 14, 2026
View reviewed changes

@SebastianKrupinski SebastianKrupinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CalDav/CardDav look correct

solracsf and others added 2 commits July 16, 2026 19:32
@solracsf @susnux
refactor(dbal): move to modern calls
fcc63b9 
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
Signed-off-by: Carl Schwan <carlschwan@kde.org>
@susnux
test: adjust expected method calls
149f9a1 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
susnux force-pushed the executeUpdateRemove branch from e5ce498 to 149f9a1 Compare July 16, 2026 17:34
@CarlSchwan
CarlSchwan merged commit 900d937 into master Jul 16, 2026
226 of 229 checks passed
@CarlSchwan
CarlSchwan deleted the executeUpdateRemove branch July 16, 2026 18:06
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 11, 2026
Merged
paulijar
paulijar reviewed Aug 12, 2026
View reviewed changes
Comment thread lib/private/DB/PreparedStatement.php
#[\Override]
public function execute($params = null): IResult {
return ($this->result = new ResultAdapter($this->statement->execute($params)));
return ($this->result = new ResultAdapter($this->statement->execute($params ?? [])));

@paulijar paulijar Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This particular change seems to have broken the PreparedStatement::bindValue and ::bindParam on NC 35.0.0beta1. In \PDOStatement::execute, the default value null has very different meaning from the empty array [] and the two should not be conflated. The empty array purges any parameters previously given with bindParam or bindValue.

I also found the branch https://github.com/nextcloud/server/tree/carl/dependabot/composer/doctrine/dbal-4.4 which looks like it would fix this issue. Is this something which is on its way to the next betas of NC35?

@CarlSchwan CarlSchwan Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the dbal 4.4 is planned for NC36, it's breaking too much stuff for merging this in NC35.

Maybe we can just revert the small change in PreparedStatement.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miaulalala miaulalala miaulalala left review comments
@paulijar paulijar paulijar left review comments
@CarlSchwan CarlSchwan CarlSchwan approved these changes
@SebastianKrupinski SebastianKrupinski SebastianKrupinski approved these changes
@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst ChristophWurst is a code owner
@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner
@nickvergessen nickvergessen Awaiting requested review from nickvergessen nickvergessen is a code owner
@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl is a code owner automatically assigned from nextcloud/server-backend
@nfebe nfebe Awaiting requested review from nfebe nfebe is a code owner automatically assigned from nextcloud/server-backend
@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend
@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh is a code owner automatically assigned from nextcloud/server-backend

Assignees

@CarlSchwan CarlSchwan

Labels

3. to review Waiting for reviews community pull requests from community feature: database Database related DB ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Milestone

Nextcloud 35

Development

Successfully merging this pull request may close these issues.

8 participants

@solracsf @miaulalala @paulijar @CarlSchwan @SebastianKrupinski @joshtrichards @susnux @nextcloud-bot