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

MsSqlServer Transaction support for dblib on Linux #534

Closed
jcwatson11 opened this issue Mar 8, 2013 · 6 comments
Closed

MsSqlServer Transaction support for dblib on Linux #534

jcwatson11 opened this issue Mar 8, 2013 · 6 comments

Comments

@jcwatson11
Copy link

I can see that the framework checks to see if dblib is present and tries to load that first, which is great! However, PDO's implementation of dblib doesn't support calling the PDO native beginTransaction() commands. I'm not sure why, but a simple work around seems to be to change the SqlServer database driver's transaction() method like so:

// inside class Illuminate\Database\SqlServerConnection

/**
 * Execute a Closure within a transaction.
 * For MsSql, transactions must be called explicitly using SQL
 * because the PDO driver does not support beginning a transaction
 * with dblib as of this writing.
 *
 * @param  Closure  $callback
 * @return mixed
 */
public function transaction(Closure $callback)
{  
    $this->pdo->exec("BEGIN TRAN");

    // We'll simply execute the given callback within a try / catch block
    // and if we catch any exception we can rollback the transaction
    // so that none of the changes are persisted to the database.
    try
    {  
        $result = $callback($this);

        $this->pdo->exec("COMMIT");
    }

    // If we catch an exception, we will roll back so nothing gets messed
    // up in the database. Then we'll re-throw the exception so it can
    // be handled how the developer sees fit for their applications.
    catch (\Exception $e)
    {  
        $this->pdo->exec("ROLLBACK TRAN");

        throw $e;
    }

    return $result;
}

I've been trying to figure out how to extend SqlServerConnection to get Laravel to connect using an overridden method without changing the underlying ConnectionResolver inside the framework. But I'm coming up empty. So maybe you could just use this method for both drivers (sqlsrv:dblib and sqlsrv:sqlsrv). I'm pretty sure it will work in both cases.

Propel seems to use this same work around for the same reason. You can read about it here:

http://propelorm.org/cookbook/using-mssql-server.html

And see their code here:

https://github.com/propelorm/Propel/blob/master/runtime/lib/adapter/MSSQL/MssqlPropelPDO.php

Thanks for an awesome framework!

If you want, I can do a fork/pull request. But it would be my first time and I won't send one unless you want it.

@bencorlett
Copy link
Contributor

On the overriding, you can override (in app/routes.php or somewhere you choose) any of the IoC offsets that are located in the DatabaseServiceProvider.

@jcwatson11
Copy link
Author

Hey Ben. Thanks for that. I'm not sure how to do that in routes. Can you give me a code example, or a general pseudo-code example or something to point me in the right direction?

Thanks!

@bencorlett
Copy link
Contributor

Sure, at the gym now, will do when I get home in an hour :)

Sent from my iPhone
Please excuse my brevity

On 11/03/2013, at 4:54 PM, jcwatson11 [email protected] wrote:

Hey Ben. Thanks for that. I'm not sure how to do that in routes. Can you give me a code example, or a general pseudo-code example or something to point me in the right direction?

Thanks!


Reply to this email directly or view it on GitHub.

@jcwatson11
Copy link
Author

Hey Ben,

Thanks for your help. I actually figured this out for myself and added another bug-fix to the pull request. There's a bug in the framework that prevents the DatabaseManager base class from reading configurations from an extension. Looks like there's no way it could have worked even if you do use $this->app['db']->extend(CLOSURE) inside your package service provider, or routes.php.

The makeConnection method was reading configuration before it knew whether we are working in the context of a package or not. And this class is also NOT using the global Config static object to resolve package names. So package database configs are not included in the results of getConfig.

See the second commit in this pull request for details on my fix. It works for me. And seems to be the direction the developer was headed when he was coding it.

Jon

@bencorlett
Copy link
Contributor

Hey, good stuff. Sorry I haven't send those examples I left my MacBook at work and typing them on an iPhone is murder.

Seems like you're on top of it!

@taylorotwell
Copy link
Member

Done.

joelharkes pushed a commit to joelharkes/framework_old that referenced this issue Mar 7, 2019
…-approval-page

Finished service approval UI + backend
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

3 participants