[FrameworkBundle][Cache] Allow configuring PDO-based cache pools, with table auto-creation on first use #27694
Conversation
8350ef5 to
8536573
Compare
|
Hmm I can imagine some scenarios where this automatic table creation could have some side-effects. For example:
Not sure how likely a scenario like this is 😄 But it could happen I guess |
|
@dmaicher sure that's possible. Is this any different than the same scenario with the table already created before, and items not saved for the same reason (a rollback?) |
I added one point above: |
|
Oh sorry I missed that part. Here are some docs on the topic: We may prefer disabling auto-creation when in the middle of a transaction... Status: needs work |
8536573 to
cbe9183
Compare
|
@dmaicher fixed. |
cbe9183 to
583eca6
Compare
| try { | ||
| $stmt = $conn->prepare($sql); | ||
| } catch (TableNotFoundException $e) { | ||
| if (!$conn->isTransactionActive() || \in_array($this->driver, array('pgsql', 'sqlite', 'sqlsrv'), true)) { |
There was a problem hiding this comment.
Ok this looks safer now.
I'm just wondering: What exactly is the use-case now for this auto-creation?
I mean we cannot really rely on it anyway on production as this will simply fail if we are in the middle of a transaction. Sure we can assume eventually there will be a request where this condition is true 😄 But as said we cannot rely on it anyway.
There was a problem hiding this comment.
Ease of use. And pgsql users will love it. Note also that even now, no exceptions are thrown when the table is not found. PSR-6 only allows a log line. This doesn't change it.
There was a problem hiding this comment.
Ah true about the muted exceptions. Forgot about that.
Yeah for pgsql indeed nice 👍
There was a problem hiding this comment.
Hi guys ! seems like issue here...
im using pgsql and TableNotFoundException throwed only when $stmt->execute();
$stmt = $conn->prepare($sql); is not throwing TableNotFoundException when cache_items table does not exists
@nicolas-grekas what do you think?
There was a problem hiding this comment.
I think I will forget about this in one hour :)
Please create an issue if you think there is something to track.
|
I'm all in for easing the usage of features ... but "database table auto-creation when the PDO cache needs it" makes me nervous. Not sure if it's a real scenario, but imagine this: I enable PDO cache in production without preparing the database for it:
|
no you don't, as explained in a comment above, PSR-6 allows no error to be reported, so this is silent - or at best logged. That's why this is an improvement over the current situation. |
…h table auto-creation on first use
583eca6 to
1484117
Compare
|
Thank you @nicolas-grekas. |
…ache pools, with table auto-creation on first use (nicolas-grekas) This PR was merged into the 4.2-dev branch. Discussion ---------- [FrameworkBundle][Cache] Allow configuring PDO-based cache pools, with table auto-creation on first use | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - * Allowed configuring PDO-based cache pools via a new `cache.adapter.pdo` abstract service * added automatic table creation when using Doctrine DBAL with PDO-based backends Commits ------- 1484117 [FrameworkBundle][Cache] Allow configuring PDO-based cache pools, with table auto-creation on first use
|
Do we need a docs issue? |
…ion (nicolas-grekas) This PR was merged into the master branch. Discussion ---------- [Cache] tell about PDO configuration and table autocreation Doc for symfony/symfony#27694 Commits ------- 187300a [Cache] tell about PDO configuration and table autocreation
cache.adapter.pdoabstract service