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

Provide access to underlying PDO #15

Open
bfinlay opened this issue May 28, 2019 · 4 comments
Open

Provide access to underlying PDO #15

bfinlay opened this issue May 28, 2019 · 4 comments

Comments

@bfinlay
Copy link

bfinlay commented May 28, 2019

The $app->db() method provided by foundation-core is very convenient to create a db instance from a dotenv configuration file. However, when you need to do something with the db connection that is not supported by PHP-DB, the only workaround is to create your own PDO connection and bypass the convenience of the $app->db().

Providing access to the PDO connect object would be useful when implementing features not supported by delight-im/PHP-DB.

The PDO connection object could be retrieved from the db object as needed for unsupported functionality, perhaps by $app->db()->getPdo()

@ocram
Copy link
Contributor

ocram commented May 29, 2019

Thanks!

Do you have any specific features in mind that are not supported by this library and thus force developers to implement a workaround (using pure PDO)?

Providing access to the internal PDO instance would obviously be quite an easy change. But there are reasons we haven’t done this so far.

We want proper encapsulation and hiding of internal data structures and APIs.

Moreover, if you have access to the internal PDO instance, you can change settings on that instance while this library relies on certain assumptions and needs these settings to remain unchanged. You could even be passing the PDO instance to third-party code which modifies certain settings.

@bfinlay
Copy link
Author

bfinlay commented May 30, 2019

My initial scenario was to use FETCH_KEY_PAIR, which is better handled as discussed here: #14

That taken care of, the primary use of getting the underlying PDO would be to pass to third party libraries. Two places where we pass the PDO in our application is 1) our custom PHP SessionHandler for storing session data, and 2) our database query library that dynamically constructs sets of related queries (not a query builder per se, but similar).

In our current use, we create the PDO connection, setup the custom PHP SessionHandler, then pass it to the PHP-AUTH component for authentication, and then pass it to the database query library.

However that bypasses the $app->db() functionality and usability.

Considering PHP's synchronous blocking and bootstrap-per-request architecture, I tend towards using a single PDO connection per request. B/C there is no performance advantage to using multiple PDO connections in a single request with PHP, but on the other hand there is potential performance disadvantage at high loads since each request is multiplied by the number of connections it needs to open. In other words 10,000 requests x 1 connection = 10,000 database connections, 10,000 requests x 2 connections = 20,000 database connections. The scale factor is not favorable.

@ocram
Copy link
Contributor

ocram commented Jun 2, 2019

Thank you!

Let’s see if I understood everything correctly:

If you want to use a single database connection – which you usually should do, as you said – this is currently possible by first creating a PDO instance manually and then passing the instance to this library, allowing for the instance to be passed to other components as well.

But this has the downside that you cannot create the instances of this library via a DataSource or Dsn, which is also used by the App#db helper from here.

So offering a method like PdoDatabase#getPdo would more or less have the same effect as the construction from a PDO instance, but with the advantage that you can use the construction via the helper classes and methods.

Is this correct?

@bfinlay
Copy link
Author

bfinlay commented Jun 4, 2019

That sounds correct, yes

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

No branches or pull requests

2 participants