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.

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

GeneratedValue strategy AUTO should use IDENTITY on PostgreSQL #5614

Closed
Seldaek opened this issue May 8, 2017 · 18 comments · Fixed by #5396 or doctrine/orm#8931
Closed

GeneratedValue strategy AUTO should use IDENTITY on PostgreSQL #5614

Seldaek opened this issue May 8, 2017 · 18 comments · Fixed by #5396 or doctrine/orm#8931

Comments

@Seldaek
Copy link
Member

Seldaek commented May 8, 2017

As per http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/basic-mapping.html#identifier-generation-strategies - AUTO uses SEQUENCE by default on PostgreSQL. This seems sub-optimal as it forces INSERT queries to provide the id explicitly using nextval('sequence_name') which is not easy to do in ORM queries, and requires that you know the sequence name. On the other hand if it's set to IDENTITY it works better as it does it like MySQL's AUTO_INCREMENT, you can just omit the id field entirely.

As doing this would cause schema changes I can imagine it won't be fixed in a minor release, but should be considered for v3 I think.

@Majkl578
Copy link
Contributor

AFAIK this would only work with SERIAL fields (fields with default value being nextval('...')). This is not default behavior and IMHO should not be considered as such.

@flaushi
Copy link

flaushi commented Mar 22, 2018

Hmm, I also stumpled upon this issue.
I have an id field with @GeneratedValue, which gives bin/console doctrine:schema:create --dump-sql:

CREATE TABLE data_sample (id INT NOT NULL, hashvalue VARCHAR(64) DEFAULT NULL, date TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL, refdate TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL, value NUMERIC(20, 2) NOT NULL, PRIMARY KEY(id));

CREATE SEQUENCE data_sample_id_seq INCREMENT BY 1 MINVALUE 1 START 1;

So the sequence is not assigned as default value. However, when adding entitities, the id is incremented. I don't understand why.

@DrummerKH
Copy link

@flaushi Because of doctrine getting ID from the sequence and add to the INSERT statement.

@breitsmiley
Copy link

breitsmiley commented Mar 12, 2019

Starting with the PostgreSQL 10 more prefered way for using autoincrement is syntax GENERATED AS IDENTITY that is very similar to mysql AUTO_INCREMENT

It would be great to support by doctrine this features

CREATE TABLE color (
    color_id INT GENERATED ALWAYS AS IDENTITY,
    color_name VARCHAR NOT NULL
);

http://www.postgresqltutorial.com/postgresql-identity-column/
https://postgrespro.com/docs/postgresql/11/sql-createtable

@menezes-
Copy link

In light of the changes introduced in PostgreSQL 10 (as mentioned by @breitsmiley) this should be reviewed. At least make the default when Postgres version equals 10

@beberlei beberlei self-assigned this Dec 8, 2020
@iamjoeker
Copy link

Any update on this one? I'm starting a new migration and it would be much welcomed as the existing database uses this. If the team is open to merging a contributed PR, I will take a stab at it.

@greg0ire
Copy link
Member

greg0ire commented Aug 9, 2021

I think until this is supported, using the IDENTITY ID generator strategy might result with SERIAL being used and this problem being solved. Would be great if somebody could confirm that.

@tchapi
Copy link

tchapi commented Aug 22, 2021

hi @greg0ire

As far as I can tell IDENTITY does not seem to work properly (doctrine/orm 2.9). I have a PostgreSQL database (10.3 and tested on 11.12 too) and a 'Department' entity that uses :

     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="IDENTITY")

And Doctrine uses sequences anyway:

❯ bin/console d:s:u --dump-sql

 The following SQL statements will be executed:

     CREATE SEQUENCE department_id_seq;
     SELECT setval('department_id_seq', (SELECT MAX(id) FROM department));
     ALTER TABLE department ALTER id SET DEFAULT nextval('department_id_seq');

So the documentation (https://www.doctrine-project.org/projects/doctrine-orm/en/2.9/reference/basic-mapping.html#identifier-generation-strategies) seems to be wrong, too, in this case (It says that PostgreSQL uses SERIAL in this case)

@greg0ire
Copy link
Member

greg0ire commented Aug 22, 2021

🤔 indeed, usesSequenceEmulatedIdentityColumns() returns true in the case of postgresql:

public function usesSequenceEmulatedIdentityColumns()
{
return true;
}
, and it has been like this since 2013: 6b4ba50

I have some faint memories of using SERIAL with postgres and Doctrine 4 years ago, and I think I was using IDENTITY at that time. This issue confirms that too: #3619

@tchapi
Copy link

tchapi commented Aug 22, 2021

Yes it's clearly a problem, and the documentation is misleading. For those that pull some hair over this, I use custom migrations with stuff like that to achieve my goals (in a Symfony app) and I don't use auto-generated migrations:

    public function up(Schema $schema): void
    {
        $this->addSql('ALTER TABLE department ALTER COLUMN id ADD GENERATED BY DEFAULT AS IDENTITY (START WITH 100)');
    }

    public function down(Schema $schema): void
    {
        $this->addSql('ALTER TABLE department ALTER id DROP IDENTITY IF EXISTS');
    }

I still leave @ORM\GeneratedValue(strategy="IDENTITY") in the Entity though. It seems to work like that

@tchapi
Copy link

tchapi commented Oct 20, 2021

Hi @beberlei, any update on this ? As explicited above, either the documentation is wrong and IDENTITY doesn't use SERIAL in PostgreSQL, or there is a bug.

Thanks a lot !

@allan-simon
Copy link
Contributor

is it

/**
* {@inheritDoc}
*/
public function getIntegerTypeDeclarationSQL(array $column)
{
if (! empty($column['autoincrement'])) {
return 'SERIAL';
}
return 'INT';
}
that should be overrided in this class https://github.com/doctrine/dbal/blob/a55eb4849b03c0c760535446cc126534e8c8c964/lib/Doctrine/DBAL/Platforms/PostgreSQL100Platform.php

by something like

    public function getIntegerTypeDeclarationSQL(array $column)
    {
        if (! empty($column['autoincrement'])) {
            return ' INT GENERATED ALWAYS AS IDENTITY';
        }

        return 'INT';
    }

?

@greg0ire
Copy link
Member

@allan-simon your proposal is something we probably should (and will?) do in the next major version of the DBAL, but in the current one, the expected result is SERIAL (as documented)

@allan-simon
Copy link
Contributor

@greg0ire , ok if it's planned on doctrine's core team side , then I should wait :)

@greg0ire
Copy link
Member

Related: #4744

@allan-simon
Copy link
Contributor

ok it seems it will land with 4.0.x of dbal

https://github.com/doctrine/dbal/pull/5396/files

@greg0ire
Copy link
Member

🤔 I'm moving this issue to doctrine/dbal then.

@greg0ire greg0ire transferred this issue from doctrine/orm Aug 23, 2022
@greg0ire greg0ire linked a pull request Aug 23, 2022 that will close this issue
@morozov morozov added this to the 4.0.0 milestone Aug 23, 2022
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.