-
Notifications
You must be signed in to change notification settings - Fork 10
Auto connect to DB on first query #84
base: master
Are you sure you want to change the base?
Conversation
code/database/driver/abstract.php
Outdated
{ | ||
if (!$this->_connection && $auto_connect) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oligriffiths Should use isConnected() here instead of this->_connection, same below.
@johanjanssens updated to use |
@oligriffiths Not sure about this one, wouldn't it make more sense to work the other way around and check if the connection is established before making the call? In what context do you need this change ? |
With no DB configured, default controller (model), invokes model, invokes table, queries DB for columns/indexes, error. This should be solved in the framework so that model controller can be used without a DB, but you shouldn't be able to query the DB with no adapter registered, should exception. Whether it auto-connects is a different matter. To me the |
The auto_connect config option is there to connect to the database when the database adapter is created. If it's not set the database doesn't auto-connect. The getConnection() method is merely a getter to return the actual connection resource, if the db is not connection this will be null. The db adapter has a isConnected() method you can use to check if a connection exists, and if not you can call connect() yourself. I don't see the reason to add auto connect behavior in getConnection() method. Everything is available in the API's to do this already. |
So, the use case here is you don't want to auto connect if you're not going to need the DB. Take a login page page or a a static about page for example that may not need any DB interaction at all. This is essentially lazy connection, only connecting when it's needed.
|
getConnection is the right place to do that, better place would be in the execute method itself. Could move the auto_connect logic and instead of connecting on construct you connect on execute if no connection exists and auto_connect is true. |
Works for me. I'll refactor.
|
Thanks, then i'm most happy to include this. |
@oligriffiths Did you ever update your PR based on the discussion we had? If not, would you mind doing so? |
What
Auto connect to DB the first time a connection is required
Why
If you don't have auto connect enabled for the driver itself, the connection isn't established when executing a query resulting in a call to a non-object