Ticket #559 (closed enhancement: fixed)
Add prepared query caching support to PropelPDO
| Reported by: | hans | Owned by: | hans |
|---|---|---|---|
| Priority: | high | Milestone: | 1.3 |
| Component: | Generator | Version: | 1.3.0beta3 |
| Severity: | major | Keywords: | |
| Cc: | cache prepared statement close cursor |
Description
From Torsten Zander on list:
We tried to implement a query cache in propel 1.3 for that we overwrote PropelPDO::prepare()
<?php private static $preparedStatements; public function prepare($sql) { if(!array_key_exists($sql, self::$preparedStatements)) { Propel::log($sql, Propel::LOG_DEBUG); return parent::prepare($sql); } else { return self::$preparedStatements($sql); } }
this works in most cases fine only if run run all tests we get the error:
SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active.
This is caused due to the fact PDO method closeCursor is not used after a fetch method.
Follow-up message from Torsten: \ I think i've got it solved with the query cache. I overwrote PropelPDO::prepare().
<?php /** * Overrides PDO::prepare() to add logging and caching * . * @param string $sql * @param array * @return PDOStatement * */ public function prepare($sql, $driver_options = array()) { $key = md5($sql); if(!array_key_exists($key, self::$preparedStatements)) { Propel::log($sql, Propel::LOG_DEBUG); return parent::prepare($sql, $driver_options); } else { return self::$preparedStatements[$key]; } }
and then I modified the PeerBuilder classes.
I added $stmt->closeCursor() after every fetch method
<?php if ($row = $stmt->fetch(PDO::FETCH_NUM)) { $stmt->closeCursor(); return (int) \$row[0];
or
<?php while ($row = $stmt->fetch(PDO::FETCH_NUM)) { } $stmt->closeCursor();
This passed our allTests with about 1300 tests and most of them using propel, so I don't see any problem. Maybe you can consider to add it to propel sources.
Change History
comment:3 Changed 3 years ago by hans
- Status changed from assigned to closed
- Resolution set to fixed
Ok, this support has been added in r912. All unit tests passing, so I don't think this will cause any problems for people.
A couple of notes:
- I changed the prepared statement cache to not be static. I wasn't sure what the side-effects of having multiple connections sharing prepared statements would be, but it seemed very likely that this would cause problems. (e.g. if same SQL statement was executed by different connections).
- I didn't notice any speed improvement with my MySQL configuration. It was actually slower with the new prepare() method (and non-emulated prepared statements). Telling the driver to emulate prepares actually sped it up a bit, and disabling the overridden prepare() statement sped it up further (back to pre-patch levels, of course). This was a very unscientific benchmarking test, however. Furthermore, my instance is not at all tuned for caching statements. I suspect that there is a performance gain to be had with this patch. Indeed, on Postgres there was a statistically significant speed improvement. My GeneratedObjectTest suite took ~5.1 seconds without this patch and ~4.7 seconds with this patch.
comment:4 Changed 3 years ago by torstenzander@…
- Status changed from closed to reopened
- Resolution fixed deleted
Hi Hans,
some how i send you the wrong method this one never really caches !!
/
- Overrides PDO::prepare() to add logging and caching
- .
- @param string $sql
- @param array
- @return PDOStatement * */
public function prepare($sql, $driver_options = array()) { $key = md5($sql);
if(!array_key_exists($key, self::$preparedStatements)) {
Propel::log($sql, Propel::LOG_DEBUG); $stmt = parent::prepare($sql, $driver_options); self::$preparedStatements[$key] = $stmt; return $stmt;
} else {
return self::$preparedStatements[$key];
}
}
sorry for that
comment:5 Changed 3 years ago by hans
- Status changed from reopened to closed
- Resolution set to fixed
No problem -- I caught that as I was applying the patch & fixed it. Take a look at [912]; let me know if you see a problem with the code I committed.
comment:6 Changed 3 years ago by hans
- Status changed from closed to reopened
- Resolution fixed deleted
I'm reopening this ticket. While the current solution does work, storing the prepared statements inside the object has the side-effect that the object will not get destroyed & so connections will not get closed by Propel::close(). This has to do with ref counts, though I'm not entirely sure I understand why :)
I'm going to disable this behavior by default but add the ability to have this enabled via an attribute in the runtime conf.
comment:9 Changed 3 years ago by hans
- Status changed from reopened to closed
- Resolution set to fixed
Ok, in r962, the behavior here was changed to make this optional. If you want to enable the prepared statement caching, you must specify a value of true for the PropelPDO::PROPEL_ATTR_CACHE_PREPARES attribute.
Example snippet from runtime-conf.xml:
<connection> <dsn>mysql:dbname=test</dsn> <attributes> <option id="PROPEL_ATTR_CACHE_PREPARES">true</option> </attributes> </connection>