Skip to content

Conversation

@kesselb
Copy link
Contributor

@kesselb kesselb commented Dec 15, 2018

When id column has no autoincrement flag query for lastInsertId fails
on postgres because no value has been generated. Call lastInsertId only
if id is null.

Could you consider a backport? Social App does not work on postgres without this like reported here #12465 (comment). Not sure how "safe" it is to backport it.

@kesselb kesselb added this to the Nextcloud 16 milestone Dec 15, 2018
@kesselb kesselb added bug 3. to review Waiting for reviews labels Dec 15, 2018
@nickvergessen
Copy link
Member

I always wonder if we should have a separate model for stuff that does nto have an autoincrement ID or no ID at all.

@rullzer
Copy link
Member

rullzer commented Dec 17, 2018

Isn't id null by default if you don't init it?

@kesselb
Copy link
Contributor Author

kesselb commented Dec 17, 2018

Isn't id null by default if you don't init it?

By default id is null

@rullzer
Copy link
Member

rullzer commented Dec 17, 2018

So then how does this fix it? As on insert of new data it will anyways be null right?

@nickvergessen
Copy link
Member

Well @rullzer the problem is the social app uses this for a model without autoincrement, the column is not even in 32bit integer anymore:
https://github.com/nextcloud/social/blob/b7ac669cb3cbd66fb36f1c1100d2508ca846863e/lib/Service/ActivityPub/NoteService.php#L143

@rullzer
Copy link
Member

rullzer commented Dec 20, 2018

Ah it is set there. OK. Yeah I think having a special entity would make sense. But for now lets do this.

When id column has no autoincrement flag query for lastInsertId fails
on postgres because no value has been generated. Call lastInsertId only
if id is null.

Signed-off-by: Daniel Kesselberg <[email protected]>
Some implementations typehint getId to integer but default is null.

Signed-off-by: Daniel Kesselberg <[email protected]>
@rullzer rullzer force-pushed the bugfix/dont-query-when-id-not-null branch from 8fcef56 to 8a952b7 Compare December 24, 2018 13:21
@rullzer rullzer merged commit bb3a7ad into master Dec 27, 2018
@rullzer rullzer deleted the bugfix/dont-query-when-id-not-null branch December 27, 2018 09:02
@kesselb
Copy link
Contributor Author

kesselb commented Dec 27, 2018

/backport to stable15

@backportbot-nextcloud
Copy link

backport to stable15 in #13278

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants