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

lastInsertId() #7

Closed
harikt opened this issue Feb 5, 2012 · 5 comments
Closed

lastInsertId() #7

harikt opened this issue Feb 5, 2012 · 5 comments
Assignees
Milestone

Comments

@harikt
Copy link
Member

harikt commented Feb 5, 2012

I was looking into the method lastInsertId ,

Postgres have a different way and I am not sure MSSQL has something different too for I don't remember or used it recently .

I feel the method public function lastInsertId($table = '' , $col = '' ) can be moved to AbstractDriver.php , so we don't repeat the same method in Mysql.php https://github.com/auraphp/Aura.Sql/blob/master/src/Aura/Sql/Driver/Mysql.php#L162 and Sqlite.php https://github.com/auraphp/Aura.Sql/blob/master/src/Aura/Sql/Driver/Sqlite.php#L228 .

public function lastInsertId($table = '' , $col = '' )
{
    $pdo = $this->getPdo();
    return $pdo->lastInsertId();
}

And we can override it from Pgsql.php https://github.com/auraphp/Aura.Sql/blob/master/src/Aura/Sql/Driver/Pgsql.php#L234 in the same fashion. Do you think its a good way ?

@ghost ghost assigned pmjones Feb 5, 2012
@harikt
Copy link
Member Author

harikt commented Feb 5, 2012

I have the commit over here harikt@3d63841 , but I need to rebase it , else there is an index.md which I have changed to add some documentation earlier which will not work in the new way :P .

@pmjones
Copy link
Member

pmjones commented Feb 5, 2012

So, the problem with overriding from an AbstractDriver is this: if you have lastInsertId($table, $col) that means you have to provide a table and column name, even if the driver doesn't need them. In MySQL and SQLite they will be ignored, but in Postgres it's important. If we just make AbstractDriver lastInsertId() and then make just the pgsql driver lastInsertId($table, $col) then you get a strictness error; the subclass signature needs to match the abstract signature. The easiest solution to make sure each driver works the way the underlying database expects it to, is to have lastInsertId() be specific to each driver.

@harikt
Copy link
Member Author

harikt commented Feb 6, 2012

May be I am missing something to understand from you .

But I wonder if we have kept lastInsertId($table = null , $col = null )
will have the same trouble ?

I did a test with

<?php
abstract class AbstractClass
{
    public function display($k = '' , $l = '' )
    {
        echo "Hello World" . PHP_EOL;
    }
}

class Hello extends AbstractClass
{
}

class World extends AbstractClass
{
    public function display($k , $l)
    {
        echo "From World" . $k . $l . PHP_EOL;
    }
}

$world = new World();
$world->display( "Yes you are right" , " You are wrong");

$hello = new Hello();
$hello->display();

output is

hari@local-lx1:/var/www/test$ php AbstractClass.php
From WorldYes you are right You are wrong
Hello World

So may be you missed to see my point . As the abstract class signature is
already having a value , but set default as null or '' , it will not be a
trouble I guess .

Did you looked the commit over
harikt@3d63841?

@pmjones
Copy link
Member

pmjones commented Feb 7, 2012

The problem with that signature is that it makes it easy for developers to think "Oh, I can get the last insert ID from any table I want." It looks wrong when applied to MySQL. Do you get what I mean there?

@harikt
Copy link
Member Author

harikt commented Feb 7, 2012

Yes understood .

@harikt harikt closed this as completed Feb 7, 2012
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