insert operations under some circumstances refused on postgreSQL (NC14)

[tuxedo-rb]

First of all, i'm inexperienced with the oddities of postgreSQL.
A user of the Dashboard App reported a problem related to postgreSQL.
nextcloud/dashboard#44

The root of the problem lies in the postgre specific lastInsertId() method.
These tries to execute:

SELECT lastval()

in class:
https://github.com/nextcloud/server/blob/master/lib/private/DB/AdapterPgSql.php#L29
for each insert operation, initiated in class:
https://github.com/nextcloud/server/blob/master/lib/public/AppFramework/Db/Mapper.php#L132

But it seems, that's under some circumstances not allowed and results in a exception.

Each App relies on AppFramework and if a App needs their own database table(s), then it uses the provided database methods.

[nextcloud-bot]

GitMate.io thinks possibly related issues are #3761 (PostgreSQL support), #9961 (PostgreSQL DeadLock), #4375 (User deletion fails in some circumstances), #8825 (PostgreSQL: Doctrine\DBAL\Exception\DriverException: An exception occurred while executing 'INSERT INTO "oc_schedulingobjects"), and #10612 (WebDAV in NC14).

[kesselb]

owncloud/core#10080

Autoincrement enabled for this table? I guess it's not possible to use this insert method on tables without autoincrement. Maybe it's not happening when u use qbmapper?

[tuxedo-rb]

Yes, autoincrement should be enabled:
https://github.com/nextcloud/dashboard/blob/stable14/appinfo/database.xml
I don't use postgreSQL, so i can't say for sure. I report only.
At least it's correct under mysql:

mysql> describe oc_dashboard_settings;
+-------+-------------+------+-----+---------+----------------+
| Field | Type        | Null | Key | Default | Extra          |
+-------+-------------+------+-----+---------+----------------+
| id    | int(11)     | NO   | PRI | NULL    | auto_increment |
| key   | varchar(45) | NO   |     | NULL    |                |
| value | int(11)     | NO   |     | 0       |                |
+-------+-------------+------+-----+---------+----------------+
3 rows in set (0,00 sec)1 row in set (0,00 sec)

QBMapper calls the same method:
https://github.com/nextcloud/server/blob/master/lib/public/AppFramework/Db/QBMapper.php#L122
Would that make any difference?

[mikaelfrykholm]

I got hit with this when trying out the new social app. I guess the ORM has a bug for pgsql. Please let me know if you want the generated ddl.

[kesselb]

Please let me know if you want the generated ddl.

Yes.

[mikaelfrykholm]

nextcloud.ddl.txt
exported it with pg_dump -s

[kesselb]

The insert operation for which table fails for you @mikaelfrykholm?

[mikaelfrykholm]

I tried creating a new post with the new ActivityPub implementation.
{"reqId":"gwolK1JSZH4bC69IXOcW","level":2,"time":"2018-12-13T12:09:12+00:00","remoteAddr":"130.239.204.245","user":"mikael","app":"no app in context","method":"POST","url":"/apps/social/api/v1/post","message":"500 - {"status":-1,"exception":"Doctrine\\DBAL\\Exception\\DriverException","message":"An exception occurred while executing 'SELECT lastval()':\n\nSQLSTATE[55000]: Object not in prerequisite state: 7 ERROR: lastval is not yet defined in this session"}","userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0","version":"15.0.0.10"}

[kesselb]

Thank you @mikaelfrykholm

You are running into owncloud/core#10080. There is no autoincrement flag set for id but the value for is defined by https://github.com/nextcloud/social/blob/b7ac669cb3cbd66fb36f1c1100d2508ca846863e/lib/Service/ActivityPub/NoteService.php#L143

1. Create a new item Note
2. Set id for item
3. Save item to database
4. Try to return the id from the database 💥 on postgres

server/lib/public/AppFramework/Db/QBMapper.php

Line 122 in 51f13a7

$entity->setId((int) $qb->getLastInsertId());

@rullzer @ChristophWurst if the id is already set we should not ask the database for an id again? not sure if this is a save change. maybe some bad code relies on this behaviour. let me know what you think.

if ($entity->getId() === null) {
    $entity->setId((int) $qb->getLastInsertId()); 
}

[ChristophWurst]

Sounds good to me.

@nickvergessen is our expert for all types of database questions 😉

[nickvergessen]

Yes, lastval only works when autoincrement was generated as far as i know.

So checking it only when id was null.

[kesselb]

15.0.1 is scheduled for 2019-10-01 and will ship a fix.

[tuxedo-rb]

Well, in the case of Dashboard branche stable14, the inserted primary key is not null.
So the reported bug is hypothetical still present, if i insert a dataset with a predefined primary key value into a table with a autoincrement column.

I won't reopen this issue, because i think the App creator chose the wrong table scheme.
Autoincrement isn't necessary for a table, if it has to handle fixed IDs.
And the provided fix should work on tables without autoincrement.

Thank you all and a happy new year.