driftphp / reactphp-dbal

DBAL for ReactPHP on top of Doctrine
https://driftphp.io
41 stars 6 forks source link

Last inserted ID #14

Open bartvanhoutte opened 4 years ago

bartvanhoutte commented 4 years ago

How do you get the last inserted ID (auto increment) after inserting something into the DB?

mmoreram commented 4 years ago

We should check how all implementations return this value and return a structure. Good point.

bartvanhoutte commented 4 years ago

In clue/reactphp-sqlite you can get the last inserted ID through $result->insertId in the resulting promise. I suggest to add a getLastInsertedId method to Result?

mmoreram commented 4 years ago

@bartvanhoutte thanks for the response! I've implemented it on mysql and sqlite, but the problem we have is Postgresql. AFAIK, postgres has some specific behavior on this, so I need to check a bit more how to do it. Any clues?

-> Temporary PR - https://github.com/driftphp/reactphp-dbal/pull/16

bartvanhoutte commented 4 years ago

@mmoreram I've just tested #16 with SQLite and it works, so looks good to me. I'm afraid I can't help you with pgsql.

Small question on the side. I see $connection->update does not return a Result but $result->fetchAllRows() instead. Is there any reason for this? This seems to be null when updating a record using SQLite.

mmoreram commented 4 years ago

@bartvanhoutte yes, you're right. We should change this behavior.

developernaren commented 4 years ago

https://stackoverflow.com/a/2944481/2159370 there is an answer for postgresql here. Let me know if I can help with anything.

developernaren commented 4 years ago
/**
     * @param string $table
     * @param array  $values
     *
     * @return PromiseInterface
     */
    public function insert(
        string $table,
        array $values
    ): PromiseInterface {

        $queryBuilder = $this
            ->createQueryBuilder()
            ->insert($table)
            ->values(array_combine(
                array_keys($values),
                array_fill(0, count($values), '?')
            ))
            ->setParameters(array_values($values));

        if($this->driver instanceof PostgreSQLDriver) {
            return $this->queryBySQL($queryBuilder->getSQL() . ' RETURNING id', $queryBuilder->getParameters());
        }

        return $this->query($queryBuilder);
    }

I tried this and this is returning correct result for insertId, I get an array with index id with correct inserted value.

developernaren commented 4 years ago
 /**
     * @param string $table
     * @param array  $values
     *
     * @return PromiseInterface
     */
    public function insert(
        string $table,
        array $values
    ): PromiseInterface {

        $queryBuilder = $this
            ->createQueryBuilder()
            ->insert($table)
            ->values(array_combine(
                array_keys($values),
                array_fill(0, count($values), '?')
            ))
            ->setParameters(array_values($values));

        if($this->driver instanceof PostgreSQLDriver) {
            $query = sprintf("SELECT COLUMN_NAME FROM information_schema.COLUMNS WHERE TABLE_NAME = '%s'", $table);
            return $this->queryBySQL($query)->then(function (Result $response) use($queryBuilder){
                $allRows = $response->fetchAllRows();
                $fields = array_map(function ($item){
                    return $item['column_name'];
                }, $allRows);

                return $this->queryBySQL($queryBuilder->getSQL() . ' RETURNING ' . implode(',', $fields), $queryBuilder->getParameters());
            });
        }

        return $this->query($queryBuilder);
    }

this seems to work perfectly, fetches the column as well.

alexmorbo commented 4 years ago

Any news to merge? Seems working fine?

mmoreram commented 4 years ago

@alexmorbo I added some extra changes on the PR - https://github.com/driftphp/reactphp-dbal/pull/19

TBH, the PostgreSQL last id implementation is not the best one, but right now, is the one I see. If you want to review it, that would be great :)

marinhekman commented 1 year ago

Is this issue not resolved already?