Repository navigation
Update Broken on PostgreSQL 10 #5930
Description
Activity
I was able to get it working by changing https://github.com/nextcloud/3rdparty/blob/f5555fef8e80d8380efb44dc8b7622a1de573c15/doctrine/dbal/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php#L292
from
$data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $this->_platform->quoteIdentifier($sequenceName));
to
$data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM pg_sequences WHERE sequencename = \'' . $sequenceName . '\'');However, this solution obviously isn't comprehensive enough for a pull request, as the pg_sequences table doesn't exist for PostgreSQL versions <10. I'm not very familiar with Doctrine, but it seems like the solution would be to check for the version, and then apply the right statement accordingly.
I had reviewed that issue, but it looks like a different issue altogether. This happens specifically because of changes to how PostgreSQL 10 handles sequence metadata.
@justin-sleep Aah ok, sorry didn't read all the specifics. Thank you for reporting!
Thanks for the report and testing with PGSQL 10 Beta 🚀
I think we should proceed as followed:
- Block PGSQL 10 on Nextcloud 9-12
- Update documentation to reflect this
- Add CI node executing tests for PGSQL 10
- Make master compatible with PGSQL 10
Still an issue in current doctrine, so this will be a bigger story:
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php#L292Docs only state 9, 9.2 and 9.4 explicitly: http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/platforms.html#postgresql
Postgres 10 was released: https://www.postgresql.org/about/news/1786/
Doctrine is now waiting for travis to add it as per doctrine/dbal#2868 ( travis-ci/travis-ci#8537 )
I just updated my nextcloud from 12.0.2 to 12.0.3, and hit this bug. Is there any way to work around it to get my nextcloud working again?
I just downgraded back.
(later i downgraded postgre and upgraded nextcloud - i was tired of update nagging)6 remaining items
For information, #5930 (comment) is still needed (and applies fine) for 12.0.4
@icewind1991 Could you have a look at this and apply the patch to our 3rdparty repo?
#7210 adds pg10 support for nc13
Reacted by Morris Jobke, enoch85, Tilo Spannagel and MickaëlCan we get this for nc12 as well? I had to rebuild a lot of containers to get it working.
Can we get this for nc12 as well? I had to rebuild a lot of containers to get it working.
The problem is, that this is an upgrade of big part of a used library. So for now it looks like we will not be able to backport this to stable12 and only support Postgres 10 on Nextcloud 13+.
Reacted by enoch85@enoch85 @MorrisJobke
Thank you for looking into it. Still it is very unfortunate that until the release of NC13 we have to stick to a broken installation.Thank you for looking into it. Still it is very unfortunate that until the release of NC13 we have to stick to a broken installation.
You changed a major part of the overall stack - doing this should always involve testing before doing it and being able to properly roll back. We will support PostgreSQL 10 with Nextcloud 13. All previous Nextcloud versions will support the PostgreSQL 9 series. Sorry for those bad news, but there is just too much risk for breakage if we would backport this.
Reacted by enoch85Reacted by Benjamin Marwell, Felipe and Ben ColeWell i was hoping to avoid issues like this by waiting for nc13 to go stable.
@MorrisJobke that's a fair comment, but please say something about the psql 10 incompatibility explicitly in the docs, for example here:
While 13 may be around the corner, quite a few installations may well be on 12 for a while and don't realize the impending disaster awaiting when the database is upgraded.
Reacted by Benjamin Marwell and Baptiste REBILLARD@MorrisJobke
I consider this a bad joke, at best.The Installation manual does not say "up to Postgres 9". It says:
Supported Platforms
Server: Linux (Debian 7, SUSE Linux Enterprise Server 11 SP3 & 12, Red Hat Enterprise Linux/CentOS 6.5 and 7 (7 is 64-bit only), Ubuntu 14.04 LTS, 16.04 LTS)
Web server: Apache 2 (mod_php, php-fpm) or Nginx (php-fpm)
Databases: MySQL/MariaDB 5.5+; PostgreSQL; Oracle 11g (currently only possible if you contact us https://nextcloud.com/enterprise as part of a subscription)Thank you very much for possibly ditching your users.
@chy-causer thanks for you other link, you were even faster. ;-)
Hi - sorry - this was not meant to trick some people into it. We just have forgotten to document it right after the PostgreSQL release. We will update the documentation accordingly.
Reacted by Benjamin Marwell, Jonas Palm and Baptiste REBILLARDSorry for the inconvenience. We updated the system requirements documentation accordingly and try to handle this better in the future.
@MorrisJobke Mistakes are part of being human. Now how could you improve your workflow to avoid such an issue in the future? Usually a Kanban board helps to avoid forgetting steps like testing, documentation, reviews, etc., every time a change is made since you have to go through all the steps.
According to PostgreSQL's 10 beta changelog
"A sequence relation now stores only the fields that can be modified by nextval(), that is last_value, log_cnt, and is_called. Other sequence properties, such as the starting value and increment, are kept in a corresponding row of the pg_sequence catalog. ALTER SEQUENCE updates are now fully transactional, implying that the sequence is locked until commit. The nextval() and setval() functions remain nontransactional.
The main incompatibility introduced by this change is that selecting from a sequence relation now returns only the three fields named above. To obtain the sequence's other properties, applications must look into pg_sequence. The new system view pg_sequences can also be used for this purpose; it provides column names that are more compatible with existing code."
Steps to reproduce
Expected behaviour
The upgrade should proceed.
Actual behaviour
I'll take a shot at fixing this later today.