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

DBAL-563: Oracle "IDENTITY" last inserted ID is returning 0 instead of the real ID #1772

Closed
doctrinebot opened this issue Jul 19, 2013 · 19 comments
Assignees
Labels
Milestone

Comments

@doctrinebot
Copy link

Jira issue originally created by user zeineddin:

I am using doctrine 2 with oracle, the tables in the database has some triggers that generate the IDs, and I am trying to us Doctrine 2 cascade persist when mapping on one-to-many, and I use "IDENTITY" in the mapping, but there is a problem which is the one-side of the relation is returning 0 as last inserted ID, which is wrong, my ID mapping of my tables is like the following:

/****

  • @Orm\Id
  • @Orm\Column(type="integer");
  • @Orm\GeneratedValue(strategy="IDENTITY")
    */
    protected $id;

and my entities looks like the following:

/****

  • @Orm\Entity

  • @Orm\Table(name="clients")
    _/
    class Client {
    /
    _

    • @Orm\Id
    • @Orm\GeneratedValue(strategy="IDENTITY")
    • @Orm\Column(type="integer")
      */
      protected $id;

    /*** @Orm\Column(name="name",type="string",length=255,unique=true) **/
    protected $name;

    /****

    • @Orm\OneToMany(targetEntity="ContactInformation", mappedBy="client", cascade={"persist"})
      ****/
      protected $contactInformations;

    public function **construct() {
    $this->contactInformations = new ArrayCollection();
    }

    public function getId() {
    return $this->id;
    }

    public function getName() {
    return $this->name;
    }

    public function setName($name) {
    $this->name = $name;
    return $this;
    }

    public function getContactInformations() {
    return $this->contactInformations;
    }

    public function addContactInformations(Collection $contactInformations)
    {
    foreach ($contactInformations as $contactInformation) {
    $contactInformation->setClient($this);
    $this->contactInformations->add($contactInformation);
    }
    }

    /****

    • @param Collection $tags
      */
      public function removeContactInformations(Collection $contactInformations)
      {
      foreach ($contactInformations as $contactInformation) {
      $contactInformation->setClient(null);
      $this->contactInformations->removeElement($contactInformation);
      }
      }

    public function setContactInformations($contactInformations) {
    $this->contactInformations = $contactInformations;
    return $this;
    }
    }

and the other entity:

/****

  • @Orm\Entity

  • @Orm\Table(name="contact_informations")
    _/
    class ContactInformation {
    /
    _

    • @Orm\Id
    • @Orm\GeneratedValue(strategy="IDENTITY")
    • @Orm\Column(type="integer")
      */
      protected $id;

    /****

    • @Orm\OneToOne(targetEntity="ContactInformationType")
    • @Orm\JoinColumn(name="type_id", referencedColumnName="id")
      ****/
      protected $type;

    /*** @Orm\Column(type="text") **/
    protected $value;

    /****

    • @Orm\ManyToOne(targetEntity="Client", inversedBy="contact_informations")
    • @Orm\JoinColumn(name="client_id", referencedColumnName="id")
      ****/
      private $client;

    public function getId() {
    return $this->id;
    }

    public function getType() {
    return $this->type;
    }

    public function setType($type) {
    $this->type = $type;
    return $this;
    }

    public function getValue() {
    return $this->value;
    }

    public function setValue($value) {
    $this->value = $value;
    return $this;
    }

    public function getClient() {
    return $this->client;
    }

    public function setClient($client = null) {
    $this->client = $client;
    return $this;
    }
    }

I also want to add: why don't Doctrine 2 just use the oracle "returning id into" statement, in this case regardless the identity mapping this will always return the inserted ID, and it will work with "AUTO", "SEQUENCE", "IDENTITY" and I think any other mapping word used!

I did try to trace where the problem come from, and it seems that when using OCI8 oracle driver that the invoked method is
Doctrine\ORM\Id\IdentityGenerator::generate
and it invokes
Doctrine\DBAL\Connection::lastInsertId
and is returning 0, I don't know why it is being invoked since the sequenceName is null (there is no sequence in the definition!)

Maybe a good solution is to check if the $statement is an 'INSERT INTO ' sql statement, then we bind an output variable to the statement which will hold the "returning ID into :output_variable" value... what do you think?

@doctrinebot
Copy link
Author

@doctrinebot
Copy link
Author

Comment created by zeineddin:

I am not professional in Doctrine, but I want to suggest something to get the last inserted id for Oracle... I think this is even better than using the sequence name to get it... and it works for all types of ID generation methods...!

I think a good solution will be to do something like this in the "Doctrine\DBAL\Driver\OCI8\OCI8Statement::**construct" (may be there is a better place or way, this is just a suggestion), and make it like the following:
1- first we check if the statement is an insert statement then we add a ' returning id into :lastInsertId' sql, here we need somehow to get the primary key column name, data type length (max_length) and make it dynamic, 'id' is just the PK in my case...
2- we bind the ':lastInsertId' parameter so that we can get it as output parameter.

here is a sample code, maybe it needs a lot of enhancement :)

public function **construct($dbh, $statement, OCI8Connection $conn)
{
list($statement, $paramMap) = self::convertPositionalToNamedPlaceholders($statement);
if (stripos($statement, 'INSERT INTO ') === 0) {
$statement = $statement . ' returning id into :lastInsertId';
}
$this->_sth = oci_parse($dbh, $statement);
$this->_dbh = $dbh;
$this->_paramMap = $paramMap;
$this->_conn = $conn;
if (stripos($statement, 'INSERT INTO ') === 0) {
oci_bind_by_name($this->_sth, ':lastInsertId', $this->lastInsertId, OCI_B_INT);
}
}

and then when executing (execute method) we will have $this->lastInsertId set for us and containing the last inserted id even if it is from a sequence... can you implement such thing? and by this the "http://docs.doctrine-project.org/en/latest/reference/basic-mapping.html#identifier-generation-strategies" Identifier Generation Strategies "IDENTITY" will work for oracle and will be full portable :)

@doctrinebot
Copy link
Author

Comment created by zeineddin:

I just attached the suggested changes in OCI8 diver files, I just need help with 2 TODO issues, and I think then all will be fine :)

@doctrinebot
Copy link
Author

Comment created by ohmytribe:

I'm suggesting this is a bug and not an improvement as it leads to different ORM behavior when using different database drivers.

@doctrinebot
Copy link
Author

Comment created by @deeky666:

Unfortunately the approach you suggested won't work as we are not able to identify the PK to return from the insert statement on the fly.

@doctrinebot
Copy link
Author

Comment created by zeineddin:

can't we get the column/s name/s from the mapping in the OCI8Statement.php file? isn't there anyway to do that? can you suggest an approad to do that? because the retuning id into :var is the right way to do that in oracle for all types of mapping!

@doctrinebot
Copy link
Author

Comment created by @deeky666:

Sure basically your approach is the way to go but I don't see a way how to determine the column name to return the last insert id for. The driver does not know what the identity column for a particular insert statement is.

@doctrinebot
Copy link
Author

Comment created by ohmytribe:

Steve, I disagree. Oracle last insert id should not rely on identity column, instead, it should rely on current sequence value. And this is properly implemented in OCI8Connection (https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php, lastInsertId method).

Besides, the proper triggers are implemented in OraclePlatform (https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Platforms/OraclePlatform.php).

As I see in OraclePlatform::getCreateAutoIncrementSql() method, the sequence name is like following:

$table . '_SEQ'

So, I think that the problem is that EntityManager (or some lower level) does not provide the proper sequence name to OCI8Connection::lastInsertId() method causing it to trigger this code:

if ($name === null) {
    return false;
}

Please, check if it helps. As for now Doctrine ORM is literally unusable with Oracle.

@doctrinebot
Copy link
Author

Comment created by ohmytribe:

Okay, seems, I've found the way it is correctly done (I had predefined sequence name, don't now if it working for entity-based generated schema):

    /****
     * @var int
     *
     * @ORM\Column(name="id", type="integer", nullable=false)
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\SequenceGenerator(sequenceName="seq_mytable", initialValue=75)
     */
    private $id;

So, we're using SEQUENCE generated value strategy for Oracle automatically and we fall back to manually defined sequence generator.

Just run persist() and flush() and got correctly set newly generated id. So cool.

@doctrinebot
Copy link
Author

Comment created by zeineddin:

This does not work for us, we are generating IDs automatically based on some triggers, so sequenceName does not work in our case... the only thing that I found working is by the modifications suggested up in the files...

@doctrinebot
Copy link
Author

Comment created by ohmytribe:

It surely has nothing to do with Oracle driver as custom id field can be generated using triggers on every database.

So, you need to implement your brand new generated value strategy as it does not comply with IDENTITY and SEQUENCE documentation. It would be a nice extension.

Maybe this could help: http://ranskills.wordpress.com/2011/05/26/how-to-add-a-custom-id-generation-strategy-to-doctrine-2-1

@doctrinebot
Copy link
Author

Comment created by zeineddin:

As you see here:
http://docs.doctrine-project.org/en/2.0.x/reference/basic-mapping.html#identifier-generation-strategies
the IDENTITY generation strategy is not implemented in Oracle... and I use it since the ID is generated bu the DB... so I think that it is better to change to the oracle way to get the last inserted ID when using this strategy...

@doctrinebot
Copy link
Author

Comment created by @deeky666:

IDENTITY generation strategy is SOMEHOW implemented in Oracle with the workaround of creating a (before insert) trigger on the specific table that uses a sequence to emulate an autoincrementation. I guess this is just a compatibility approach for IDENTITY strategy on a best effort basis and should not be relied on. This is also the reason why it is stated in the documentation as not fully portable. The issue discussed here is also not an issue of Doctrine ORM IMO as it is not responsible for evaluating if an IDENTITY strategy needs a sequence for the underlying driver to obtain the last insert ID. However there already seems to be hack for exactly the same case in PostgreSQL. See:

https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L453

What we probaby COULD do is add another check in the ClassMetadataFactory for the Oracle platform to tell it to use a sequence for IDENTITY strategy. But that still is rather hackish to be honest...

@doctrinebot
Copy link
Author

Comment created by @beberlei:

[~deeky666] The real issue is indeed, that IDENTITY is not really supported for Oracle. We would need to find a way to support it generically or throw an exception in the ORM if Oracle is used with IDENTITY.

@doctrinebot
Copy link
Author

Comment created by @deeky666:

Step one in fixing this issue has been applied in PR #428 and fixed in commit d284525.
Step two has been supplied in ORM in PR doctrine/orm#890.

As soon as the PR on ORM side gets merged it is possible to use IDENTITY generator strategy with Oracle :)

@doctrinebot
Copy link
Author

Comment created by @doctrinebot:

A related Github Pull-Request [GH-890] was closed:
doctrine/orm#890

@doctrinebot
Copy link
Author

Comment created by @deeky666:

Fixed in commits:
d284525
doctrine/orm@a7b9140

@doctrinebot
Copy link
Author

Issue was closed with resolution "Fixed"

@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 Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants