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

🐘 Support postgres15 #34645

Merged
merged 1 commit into from
Nov 14, 2022
Merged

🐘 Support postgres15 #34645

merged 1 commit into from
Nov 14, 2022

Conversation

vitormattos
Copy link
Contributor

@vitormattos vitormattos commented Oct 18, 2022 •
edited
Loading

close #34617

Tested with Postgres 12, 13, 14 and 15 and worked fine.

@vitormattos vitormattos added the 3. to review Waiting for reviews label Oct 18, 2022
@vitormattos vitormattos added this to the Nextcloud 26 milestone Oct 18, 2022
@vitormattos vitormattos self-assigned this Oct 18, 2022
@szaimen szaimen added the pending documentation This pull request needs an associated documentation update label Oct 18, 2022
@vitormattos vitormattos mentioned this pull request Oct 18, 2022
Closed
9 tasks
szaimen
szaimen approved these changes Oct 18, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

lgtm now

@nickvergessen nickvergessen changed the title Bugfix to support postgres15 🐘 Support postgres15 Oct 18, 2022
@nickvergessen
Copy link
Member

So this fixes new installations. Any chance that we can fix existing ones on update?

@szaimen
Copy link
Contributor

szaimen commented Oct 18, 2022 •
edited
Loading

So this fixes new installations. Any chance that we can fix existing ones on update?

Not sure, they should get broken upon upgrade, IIRC?

Or if they are because they upgrade to Postgresql 15 wrongly, you need to run GRANT ALL PRIVILEGES ON SCHEMA public TO $DB_USER once which we cannot do if our user is not postgresql admin which it also should not be anyway.

@nickvergessen
Copy link
Member

Or if they are because they upgrade to Postgresql 15 wrongly, you need to run GRANT ALL PRIVILEGES ON SCHEMA public TO $DB_USER once which we cannot do if our user is not postgresql admin which it also should not be anyway.

But we could show this as an error with the correct user already filled in? Instead of "Insufficient privileges" (not sure if we can detect it #34617 (comment) )

@szaimen
Copy link
Contributor

szaimen commented Oct 18, 2022

docs improvements in nextcloud/documentation#9227

@vitormattos vitormattos mentioned this pull request Oct 22, 2022
Merged
@szaimen
Copy link
Contributor

szaimen commented Oct 24, 2022 •
edited
Loading

Rebase after #34736

@szaimen szaimen requested review from a team and CarlSchwan and removed request for a team October 24, 2022 16:49
juliushaertl
juliushaertl previously requested changes Oct 26, 2022
View reviewed changes
Copy link
Member

@juliushaertl juliushaertl left a comment

Choose a reason for hiding this comment

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

Can we add drone jobs for postgres 14 and 15 as well?

10 can probably be dropped for master as it will have its final release this November according to https://www.postgresql.org/support/versioning/ but not sure about how we decided about dropping database versions in the past.

@nickvergessen
Copy link
Member

but not sure about how we decided about dropping database versions in the past.

I would argue the same way as with PHP. The next major after it was dropped from upstream we should remove it. So +1 on dropping 10 with Nextcloud 26

@szaimen
Copy link
Contributor

szaimen commented Oct 27, 2022

Can we add drone jobs for postgres 14 and 15 as well?

10 can probably be dropped for master as it will have its final release this November according to https://www.postgresql.org/support/versioning/ but not sure about how we decided about dropping database versions in the past.

@juliushaertl can we do that in a follow-up PR?

@szaimen
Copy link
Contributor

szaimen commented Oct 27, 2022

/rebase

szaimen
szaimen reviewed Nov 11, 2022
View reviewed changes
.drone.yml Outdated Show resolved Hide resolved
@szaimen
Copy link
Contributor

szaimen commented Nov 11, 2022

It works 🎉🎉🎉

@vitormattos @nickvergessen @szaimen
Bugfix to support postgres15
eb7b682 
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Simon L <szaimen@e.mail.de>
Co-Authored-By: Joas Schilling <213943+nickvergessen@users.noreply.github.com>
@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 11, 2022
@szaimen
Copy link
Contributor

szaimen commented Nov 11, 2022

/backport to stable25

@szaimen szaimen mentioned this pull request Nov 11, 2022
Merged
@szaimen szaimen merged commit f1b940b into master Nov 14, 2022
@szaimen szaimen deleted the bugfix/support-to-postgres15 branch November 14, 2022 09:02
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Nov 14, 2022
Merged
@IrwenXYZ
Copy link

hi guys, thanks for the quick fix for psql 15. I'm planning on setting up a Nextcloud instance, and was wondering if a Nextcloud 20.0.1 instance can upgrade gracefully from Postgres 14 to 15 through this PR?

alternatively, should I was for 25.0.2 to release to set it up?

thank you!

@nickvergessen
Copy link
Member

Not sure if it is a typo, but 20.0.1 is not updated and not supported anymore. If you mean 25.0.1 the answer is No, you should wait for 25.0.2 which is planned to be released soon, (RC1 was released last week and then it usually doesn't take a long time).

@IrwenXYZ
Copy link

IrwenXYZ commented Dec 2, 2022

Yes I meant 25.0.2. Thank you for the answer, I will wait for 25.0.2!

nickvergessen added a commit to nextcloud/spreed that referenced this pull request Dec 2, 2022
@nickvergessen
Adjust postgres database access
2690549 
Following server example from
nextcloud/server#34645

Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen added a commit to nextcloud/spreed that referenced this pull request Dec 6, 2022
@nickvergessen
Adjust postgres database access
175b054 
Following server example from
nextcloud/server#34645

Signed-off-by: Joas Schilling <coding@schilljs.com>
backportbot-nextcloud bot pushed a commit to nextcloud/spreed that referenced this pull request Dec 6, 2022
@nickvergessen @backportbot-nextcloud
Adjust postgres database access
52d3e97 
Following server example from
nextcloud/server#34645

Signed-off-by: Joas Schilling <coding@schilljs.com>
backportbot-nextcloud bot pushed a commit to nextcloud/spreed that referenced this pull request Dec 6, 2022
@nickvergessen @backportbot-nextcloud
Adjust postgres database access
e5a0b7d 
Following server example from
nextcloud/server#34645

Signed-off-by: Joas Schilling <coding@schilljs.com>
@DaphneMuller
Copy link

hello @vitormattos @szaimen ,
Thank you for your work on this pull request! This ticket seems to have the tag 'missing documentation', is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@szaimen szaimen removed the pending documentation This pull request needs an associated documentation update label Feb 21, 2023
@szaimen
Copy link
Contributor

szaimen commented Feb 21, 2023

documentation is done already

@joshtrichards joshtrichards mentioned this pull request May 19, 2023
Closed
9 tasks
This was referenced Jun 12, 2023
Closed
Merged
@joshtrichards joshtrichards mentioned this pull request Jun 18, 2024
Closed
8 tasks
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

@szaimen szaimen szaimen approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

Assignees

@vitormattos vitormattos

Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: install and update
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.

[Bug]: NC 25 RC5 not compatible with postgresql 15
10 participants
@vitormattos @nickvergessen @szaimen @juliushaertl @IrwenXYZ @DaphneMuller @PVince81 @ChristophWurst @kesselb @gaudryc