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

Wrong parameter in AbstractTable::insertRowPerform() for lastInsertId method at PostgreSQL #71

Closed
froschdesign opened this issue Aug 17, 2017 · 10 comments

Comments

@froschdesign
Copy link
Contributor

At a PostgreSQL database the name of a sequence object is required for PDO::lastInsertId. At the moment the name of the autoincrement column is used.

$autoinc = $this->getAutoinc();
if ($autoinc) {
$row->$autoinc = $connection->lastInsertId($autoinc);
}

Changing the getAutoinc method in the specific table class doesn't work, because the method AbstractTable::insertRowPrepare() needs the name of the autoincrement column.

$autoinc = $this->getAutoinc();
if ($autoinc) {
unset($cols[$autoinc]);
}

@pmjones
Copy link
Contributor

pmjones commented Aug 17, 2017

To make sure I understand: are you saying you need a sequence name other than the autoincrement column?

@froschdesign
Copy link
Contributor Author

froschdesign commented Aug 17, 2017

@pmjones

are you saying you need a sequence name other than the autoincrement column?

Right, the name of a sequence. (http://php.net/manual/en/pdo.lastinsertid.php#refsect1-pdo.lastinsertid-description)

My current workaround in the mapper:

if ($record->getRow()->getStatus() === Row::INSERTED) {
    /** @var \Aura\Sql\ExtendedPdoInterface $connection */
    $connection = $this->getTable()->getWriteConnection();
    $lastId     = $connection->lastInsertId('foobar_id_seq');
}

(Thanks for the quick response!)

@pmjones
Copy link
Contributor

pmjones commented Aug 17, 2017

I could have sworn I had something like that workaround implemented in the Insert object(s) for Postgres. Let me take a look around. Sorry for the hassle.

@pmjones
Copy link
Contributor

pmjones commented Aug 21, 2017

Turns out what I have is in Aura.SqlQuery, and it's for extended Postgres tables, not non-extended ones. Working on a fix now. Thanks for your patience.

@pmjones
Copy link
Contributor

pmjones commented Aug 21, 2017

Update: I did implement it in Aura.SqlQuery:

https://github.com/auraphp/Aura.SqlQuery/blob/3.x/src/Pgsql/Insert.php#L47-L54

But Atlas doesn't call getLastInsertIdName, it just presumes the name is the column name. Continuing to work on it.

@pmjones
Copy link
Contributor

pmjones commented Aug 21, 2017

@froschdesign Do me a favor? In your local copy of Atlas\Orm\Table\AbstractTable::insertRowPerform() change from this:

        if ($autoinc) {
            $row->$autoinc = $connection->lastInsertId($autoinc);
        }

To this:

        $autoinc = $this->getAutoinc();
        if ($autoinc) {
            $lastInsertIdName = $insert->getLastInsertIdName($autoinc);
            $row->$autoinc = $connection->lastInsertId($lastInsertIdName);
        }

And tell me if it works for you.

@pmjones
Copy link
Contributor

pmjones commented Aug 21, 2017

@froschdesign see also #72 as a fix

@froschdesign
Copy link
Contributor Author

@pmjones
Sorry for the late response.

And tell me if it works for you.

Works like a charm! 👍

@pmjones
Copy link
Contributor

pmjones commented Aug 22, 2017

Excellent! I have applied the fix on the 1.x branch; use that, until the next release comes out (which I hope to do later today).

@froschdesign
Copy link
Contributor Author

Great! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants