Server: Update Broken on PostgreSQL 10

[justin-sleep]

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

1. Run PostgreSQL 10 beta
2. Upgrade Nextcloud (likely to any version)

Expected behaviour

The upgrade should proceed.

Actual behaviour

Doctrine\DBAL\Exception\InvalidFieldNameException: An exception occurred while executing 'SELECT min_value, increment_by FROM "oc_activity_mq_mail_id_seq"':

SQLSTATE[42703]: Undefined column: 7 ERROR:  column "min_value" does not exist
LINE 1: SELECT min_value, increment_by FROM "oc_activity_mq_mail_id_...
               ^
Update failed

I'll take a shot at fixing this later today.

[justin-sleep]

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.

[enoch85]

This is already known: https://github.com/nextcloud/server/issues/5909

[justin-sleep]

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.

[enoch85]

@justin-sleep Aah ok, sorry didn't read all the specifics. Thank you for reporting!

[LukasReschke]

Thanks for the report and testing with PGSQL 10 Beta 🚀

I think we should proceed as followed:

• [ ] Block PGSQL 10 on Nextcloud 9-12
• [x] Update documentation to reflect this
• [x] Add CI node executing tests for PGSQL 10
• [x] Make master compatible with PGSQL 10

[nickvergessen]

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#L292

Docs only state 9, 9.2 and 9.4 explicitly: http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/platforms.html#postgresql

[nickvergessen]

Postgres 10 was released: https://www.postgresql.org/about/news/1786/

[nickvergessen]

Doctrine is now waiting for travis to add it as per doctrine/dbal#2868 ( travis-ci/travis-ci#8537 )

[douglas-carmichael]

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?

[Anrock]

I just downgraded back.
(later i downgraded postgre and upgraded nextcloud - i was tired of update nagging)

[douglas-carmichael]

I was able to get nextCloud 12.0.3 working with PostgreSQL 10 by using this patch from DBAL issue 2868 (https://github.com/doctrine/dbal/issues/2868):

    $version = floatval($this->_conn->getWrappedConnection()->getServerVersion());

    if ($version >= 10) {
       $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM pg_sequences WHERE schemaname = \'public\' AND sequencename = '.$this->_conn->quote($sequenceName));
    }
    else
    {
        $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $this->_platform->quoteIdentifier($sequenceName));
    }

Would it be possible to integrate this patch into the nextCloud distribution?
It works fine from both the web and desktop clients (and I was also able to do 'occ update' without an error message), but nextCloud complains about the code integrity check failing.

[nickvergessen]

The biggest issue is we can not test this on travis atm, neither can doctrine, which is why the patch is not merged yet. My guess is we will not put this into 12, but maybe in 13.

[lrupp]

I can confirm that the patch from douglas works here on my Nextcloud 12.0.3 with PostgreSQL 10.0. Here is a "diff -u" for those who also want to try it out:

--- 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php.orig  2017-11-05 15:37:27.538064270 +0100
+++ 3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php 2017-11-05 15:38:54.014644323 +0100
@@ -289,7 +289,16 @@
             $sequenceName = $sequence['relname'];
         }

-        $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $this->_platform->quoteIdentifier($sequenceName));
+        $version = floatval($this->_conn->getWrappedConnection()->getServerVersion());
+
+        if ($version >= 10) {
+           $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM pg_sequences WHERE schemaname = \'public\' AND sequencename = '.$this->_conn->quote($sequenceName));
+        }
+        else
+        {
+            $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $this->_platform->quoteIdentifier($sequenceName));
+        }
+//        $data = $this->_conn->fetchAll('SELECT min_value, increment_by FROM ' . $this->_platform->quoteIdentifier($sequenceName));

         return new Sequence($sequenceName, $data[0]['increment_by'], $data[0]['min_value']);
     }

[nerzhul]

if you can port it to 12.0.x can be nice as pg10 is out since 1 month and some of us started to use it :)

[bmarwell]

Hello.

I just ported my NC installation to docker. I obviously started with the most recent stable versions, which are NC 12 and Postgres 10. Now I see that some apps (e.g. my bookmark apps) aren't working. This issue affects me already. As the container is immutable, please provide a workaround. Mounting the fixed file as volume? Something similar? Thanks in advance.

[lwandrebeck]

For information, https://github.com/nextcloud/server/issues/5930#issuecomment-341978185 is still needed (and applies fine) for 12.0.4

[MorrisJobke]

@icewind1991 Could you have a look at this and apply the patch to our 3rdparty repo?

[icewind1991]

https://github.com/nextcloud/server/pull/7210 adds pg10 support for nc13

[JGjorgji]

Can we get this for nc12 as well? I had to rebuild a lot of containers to get it working.

[MorrisJobke]

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+.

[enoch85]

only support Postgres 10 on Nextcloud 13+.

Why do you want to stay on an old version? NC 13 brings so much improvements. I'd say one more reason to upgrade @JGjorgji :)

[bmarwell]

@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.

[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.

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.

[JGjorgji]

Well i was hoping to avoid issues like this by waiting for nc13 to go stable.

[chy-causer]

@MorrisJobke that's a fair comment, but please say something about the psql 10 incompatibility explicitly in the docs, for example here:

https://docs.nextcloud.com/server/12/admin_manual/configuration_database/linux_database_configuration.html

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.

[bmarwell]

@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)

_Source:_
https://docs.nextcloud.com/server/12/admin_manual/installation/system_requirements.html#supported-platforms

Thank you very much for possibly ditching your users.

@chy-causer thanks for you other link, you were even faster. ;-)

[MorrisJobke]

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.

[MorrisJobke]

Sorry for the inconvenience. We updated the system requirements documentation accordingly and try to handle this better in the future.

https://docs.nextcloud.com/server/12/admin_manual/installation/system_requirements.html#supported-platforms

[cluxter]

@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.