Skip to content

[stable21] Fix Oracle query limit compliance in Comments#27203

Merged
blizzz merged 3 commits intostable21from
backport/27187/stable21
Sep 27, 2021
Merged

[stable21] Fix Oracle query limit compliance in Comments#27203
blizzz merged 3 commits intostable21from
backport/27187/stable21

Conversation

@backportbot-nextcloud
Copy link

@backportbot-nextcloud backportbot-nextcloud bot commented May 28, 2021

backport of #27187

@backportbot-nextcloud backportbot-nextcloud bot added this to the Nextcloud 21.0.3 milestone May 28, 2021
@skjnldsv skjnldsv added the 4. to release Ready to be released and/or waiting for tests to finish label May 28, 2021
nickvergessen
nickvergessen reviewed May 31, 2021
View reviewed changes
lib/private/Comments/Manager.php Outdated
$query->setParameter('ids', $chunk, IQueryBuilder::PARAM_INT_ARRAY);
$result = $query->execute();

$unreadComments += array_fill_keys($objectIds, 0);
Copy link
Member

@nickvergessen nickvergessen May 31, 2021

Choose a reason for hiding this comment

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

since this uses $objectIds instead of $chunk it would have been enough to do it once before the loop even starts?

Copy link
Member

@Simounet Simounet May 31, 2021

Choose a reason for hiding this comment

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

You're right. Do you want me to make a new PR?

Copy link
Member

@nickvergessen nickvergessen May 31, 2021

Choose a reason for hiding this comment

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

We can fix it here and do a new PR for master only

Copy link
Member

@Simounet Simounet Jun 1, 2021

Choose a reason for hiding this comment

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

Ok, let me know if I can do anything.

Copy link
Member

@nickvergessen nickvergessen Jun 1, 2021

Choose a reason for hiding this comment

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

Fixed it, master PR in #27316

MorrisJobke
MorrisJobke reviewed Jun 7, 2021
View reviewed changes
@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Jun 7, 2021
@blizzz blizzz mentioned this pull request Jun 23, 2021
Merged
9 tasks
@blizzz blizzz force-pushed the backport/27187/stable21 branch from 128df0c to 736898e Compare June 23, 2021 23:42
@magikmw magikmw mentioned this pull request Jun 24, 2021
Closed
@blizzz blizzz mentioned this pull request Jul 1, 2021
Merged
10 tasks
@blizzz
Copy link
Member

blizzz commented Jul 1, 2021

CI is still unhappy

@ChristophWurst ChristophWurst mentioned this pull request Jul 5, 2021
Closed
@AndyXheli
Copy link
Contributor

AndyXheli commented Jul 6, 2021

Looks like this has been fixed in NC22 but still an issue on 21.0.3

@skjnldsv skjnldsv mentioned this pull request Jul 26, 2021
Merged
14 tasks
@skjnldsv
Copy link
Member

skjnldsv commented Jul 27, 2021

@Simounet what is the status here?

@Simounet
Copy link
Member

Simounet commented Jul 27, 2021

@skjnldsv I would love to help you here. I didn't know that you were waiting for me.

@skjnldsv skjnldsv mentioned this pull request Aug 3, 2021
Merged
12 tasks
@skjnldsv skjnldsv removed this from the Nextcloud 21.0.4 milestone Aug 5, 2021
@skjnldsv skjnldsv added this to the Nextcloud 21.0.5 milestone Aug 5, 2021
@fblz
Copy link

fblz commented Aug 22, 2021

Still an issue in 21.0.4

@szaimen
Copy link
Contributor

szaimen commented Aug 31, 2021

/rebase

@szaimen szaimen self-assigned this Aug 31, 2021
artonge
artonge requested changes Sep 15, 2021
View reviewed changes
@szaimen szaimen removed their assignment Sep 15, 2021
@blizzz blizzz mentioned this pull request Sep 23, 2021
Merged
8 tasks
@blizzz
Copy link
Member

blizzz commented Sep 23, 2021

CI unhappy

@szaimen
Copy link
Contributor

szaimen commented Sep 23, 2021

/rebase

Simounet and others added 2 commits September 23, 2021 08:37
@Simounet @szaimen
Fix Oracle query limit compliance in Comments
f325a4a 
Signed-off-by: Simounet <contact@simounet.net>
@nickvergessen @szaimen
Fix populating the array and closing the cursors
8e7eea9 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@szaimen
Copy link
Contributor

szaimen commented Sep 23, 2021

@artonge do you mind fixing dco for your commit? I think CI should otherwise be green now :)

@artonge
Use legacy execute method
1297b04 
Use legacy execute method instead of executeQuery introduced in 22

Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge force-pushed the backport/27187/stable21 branch from 81bdc4d to 1297b04 Compare September 23, 2021 10:45
@szaimen
Copy link
Contributor

szaimen commented Sep 23, 2021 •
edited
Loading

@artonge thanks! :)

@szaimen
Copy link
Contributor

szaimen commented Sep 23, 2021 •
edited
Loading

failure is unrelated as far as I can see

@blizzz do you think that merging this now (after the RC) is feasible?

@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Sep 26, 2021
@blizzz
Copy link
Member

blizzz commented Sep 27, 2021

@blizzz do you think that merging this now (after the RC) is feasible?

Case by case. Since the original change was already shipped with 22 and because it is not a complex change, I think it's ok.

@blizzz blizzz merged commit 62a5d27 into stable21 Sep 27, 2021
@blizzz blizzz deleted the backport/27187/stable21 branch September 27, 2021 09:02
@blizzz blizzz mentioned this pull request Sep 29, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@artonge artonge artonge requested changes

@skjnldsv skjnldsv skjnldsv approved these changes

@blizzz blizzz Awaiting requested review from blizzz

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@PVince81 PVince81 Awaiting requested review from PVince81

@Simounet Simounet Awaiting requested review from Simounet

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Milestone

Nextcloud 21.0.5

Development

Successfully merging this pull request may close these issues.

9 participants

@blizzz @AndyXheli @skjnldsv @Simounet @fblz @szaimen @nickvergessen @MorrisJobke @artonge