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.

Sign up for GitHub

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

Jump to bottom

On folder move execute only one UPDATE query for all nested items. #6211

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

On folder move execute only one UPDATE query for all nested items. #6211

wants to merge 15 commits into from
+270 −48

Conversation

allexzander
Copy link
Contributor

So, the idea here is to not execute INSERT query for every nested item of moved folder (there can be thousands of items and this is very very slow and also brings an issue of user moving/renaming the same folder multiple times, causing local database records to become messed up and virtual files being removed on a later sync). Instead, the UPDATE query is executed, which is very fast. Tested on 1000 files so far so good.
Let's keep it draft for now until I have time to properly finalize it.

Should fix #4636 or at least make it much less likely to happen.

@allexzander allexzander force-pushed the feature/single-sql-query-on-move branch 2 times, most recently from 9b38399 to 982df39 Compare November 13, 2023 16:29
@allexzander allexzander reopened this Nov 15, 2023
@allexzander allexzander force-pushed the feature/single-sql-query-on-move branch from 42276df to 08e1958 Compare November 16, 2023 00:31
@sonarqubecloud SonarQubeCloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell B 40 Code Smells

83.5% 83.5% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@allexzander allexzander force-pushed the feature/single-sql-query-on-move branch from ccc72ba to cb9e5ac Compare January 10, 2024 15:58
@allexzander allexzander force-pushed the feature/single-sql-query-on-move branch from 147c170 to 35e5446 Compare January 29, 2024 20:05
@allexzander allexzander marked this pull request as ready for review January 29, 2024 20:12
@allexzander allexzander force-pushed the feature/single-sql-query-on-move branch from 35e5446 to 18008eb Compare February 14, 2024 20:54
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
64 New Code Smells (required ≤ 0)
C Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@allexzander allexzander mentioned this pull request Feb 18, 2024
Closed
@allexzander allexzander force-pushed the feature/single-sql-query-on-move branch from 18008eb to 122ae9c Compare April 24, 2024 18:27
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
C Maintainability Rating on New Code (required ≥ A)
74 New Code Smells (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@allexzander @mgallien
On folder move execute only one UPDATE query for all nested items.
ee875f7 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Iteration. Fixing tests.
84a39f2 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Iteration. Improved move optimization logic. Also account for rename …
1769a7d 
…target such that it is the same move.

Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
iteration.
9828480 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Iteration.
0342f57 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Fixed incorrect move logic and tests.
e3cc128 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Remove unnecessary changes.
0d7826f 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Refactoring.
61767de 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Do not update pinstates and inode on move for WindowsCfApi mode.
2c8ec7e 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Refactoring.
338bd88 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Fix compile issues.
6b26b01 
Signed-off-by: alex-z <[email protected]>
@allexzander @mgallien
Added some todos.
21e347e 
Signed-off-by: alex-z <[email protected]>
@mgallien mgallien force-pushed the feature/single-sql-query-on-move branch from 122ae9c to 2a7adfe Compare August 13, 2024 07:44
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-6211-2a7adfe766f6258e420c2b51dee79447684c40c2-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
C Maintainability Rating on New Code (required ≥ A)
81 New Code Smells (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@Rello Rello added this to the 3.16.0 milestone Dec 20, 2024
mgallien
mgallien requested changes Feb 10, 2025
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

conflicts need to be fixed
most probably tests should be done (maybe automated and manual) to ensure no regressions

@camilasan camilasan modified the milestones: 3.16.0, 3.17.0 Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien requested changes

@claucambra claucambra claucambra approved these changes

@camilasan camilasan Awaiting requested review from camilasan

Requested changes must be addressed to merge this pull request.

Assignees

@mgallien mgallien

Labels
None yet
Projects
None yet
Milestone
3.17.0
Development

Successfully merging this pull request may close these issues.

[Bug]: Some virtual files cannot be sync'd: "Conflict when uploading a file. It's going to get removed!"
6 participants
@allexzander @nextcloud-desktop-bot @mgallien @claucambra @camilasan @Rello