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

do not forget DB table prefix with truncate query #31620

Merged
merged 1 commit into from
Mar 22, 2022
Merged

do not forget DB table prefix with truncate query #31620

merged 1 commit into from
Mar 22, 2022

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Mar 18, 2022

  • as used in LDAP's AbstractMapping::clear() method
  • and in Comment's ManagerTest::setUp()
  • fixes a DB Exception with Oracle

@blizzz
do not forget DB table prefix with truncate query
4656753 
- as used in LDAP's AbstractMapping::clear() method
- and in Comment's ManagerTest::setUp()
- fixes a DB Exception with Oracle

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz added this to the Nextcloud 24 milestone Mar 18, 2022
@blizzz blizzz requested review from nickvergessen, come-nc, a team and PVince81 and removed request for a team March 18, 2022 16:04
@come-nc
Copy link
Contributor

come-nc commented Mar 21, 2022

How does one know when PREFIX is needed and when not?
We have other calls on the IDBConnection that do not contain it:
https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php#L184

@blizzz
Copy link
Member Author

blizzz commented Mar 21, 2022

How does one know when PREFIX is needed and when not? We have other calls on the IDBConnection that do not contain it: https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/Migration/Version1130Date20211102154716.php#L184

It is not needed when using the query builder.

nickvergessen
nickvergessen approved these changes Mar 21, 2022
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

That doc block thou 🤪

Maybe we fix that properly?

@PVince81 PVince81 merged commit 6fa51be into master Mar 22, 2022
@PVince81 PVince81 deleted the fix/noid/truncate-oracle branch March 22, 2022 09:46
@blizzz

This comment was marked as resolved.

Sign in to view
@blizzz

This comment was marked as duplicate.

Sign in to view
@blizzz
Copy link
Member Author

blizzz commented Mar 22, 2022

/backport to stable21

@blizzz
Copy link
Member Author

blizzz commented Mar 22, 2022

/backport to stable20

@backportbot-nextcloud

This comment was marked as resolved.

Sign in to view
@backportbot-nextcloud

This comment was marked as duplicate.

Sign in to view
@backportbot-nextcloud
Copy link

The backport to stable20 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable21 failed. Please do this backport manually.

@blizzz
Copy link
Member Author

blizzz commented Mar 22, 2022

/backport to stable23

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Mar 22, 2022
Merged
@blizzz

This comment was marked as duplicate.

Sign in to view
@blizzz

This comment was marked as duplicate.

Sign in to view
@blizzz

This comment was marked as duplicate.

Sign in to view
@backportbot-nextcloud

This comment was marked as duplicate.

Sign in to view
@backportbot-nextcloud

This comment was marked as resolved.

Sign in to view
@backportbot-nextcloud

This comment was marked as resolved.

Sign in to view
@blizzz
Copy link
Member Author

blizzz commented Mar 22, 2022

/backport to stable22

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Mar 22, 2022
Merged
@blizzz
Copy link
Member Author

blizzz commented Mar 22, 2022

/backport to stable21

@blizzz

This comment was marked as duplicate.

Sign in to view
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Mar 22, 2022
Merged
@backportbot-nextcloud

This comment was marked as duplicate.

Sign in to view
@blizzz
Copy link
Member Author

blizzz commented Mar 23, 2022

/backport to stable20

@backportbot-nextcloud
Copy link

The backport to stable20 failed. Please do this backport manually.

@blizzz blizzz mentioned this pull request Mar 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@come-nc come-nc come-nc approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug feature: ldap
Projects
None yet
Milestone
Nextcloud 24
Development

Successfully merging this pull request may close these issues.
5 participants
@blizzz @come-nc @nickvergessen @PVince81 @skjnldsv