On Sat, Jan 26, 2013 at 07:21:56PM +1100, Peter Rabbitson wrote:
> On Fri, Jan 25, 2013 at 10:59:46PM +0000, Tim Bunce wrote:
> > On Fri, Jan 25, 2013 at 09:40:07PM +1100, Peter Rabbitson wrote:
> > > On Fri, Jan 25, 2013 at 10:17:11AM +0000, Tim Bunce wrote:
> > > > Re https://rt.cpan.org/Public/Bug/Display.html?id=82942
> > > >
> > > > The key question: is this a bug or a feature?
> > >
> > > Can you walk me through your thought process on what makes this a feature?
> > > Not trying to be difficult, I am just having trouble seeing the use-case
> > > ;)
> >
> > A cache statement is cached because the user asked for it to be cached.
>
> Perhaps there is a mismatch of understanding here. For me the only
> benefit of using a cached statement is to avoid a prepare. Executing the
> *same* statement with the *same* values *without* rebind does not seem
> to be possible in the first place (according to the docs at least):
I don't follow you here Peter. This should work fine:
$sth = $dbh->prepare_cached($sql);
$sth->bind_param(1, $foo);
$sth->execute();
$sth->execute();
$sth->execute();
$sth->execute();
> > $sth = $dbh->prepare_cached($sql);
> >
> > I think it's reasonable to assume that binding a reference value to a
> > placeholder of a cached statement handle will cause the ref count of the
> > value to increase.
> >
> > $sth->bind_param_inout(1, \@foo);
>
> Yes, otherwise it may get lost before the consequent execute().
> > So, the question then is what options do we have for "fixing" it?
> >
> > The most obvious thing to do would be to weaken the refs in
> > values %{ $sth->{ParamValues} }. That could be done in the
> > application, so a DBI change isn't strictly needed.
>
> That won't work as per my above remark. Now that I think of it your
> cached statements are not copies, they are extra refs. Which means that
> you can't really weaken it in the cached ones only.
>
> > If it's agreed that the DBI should weaken the refs then
> > we'd need to discuss when. As soon as they're bound?
> > I can imagine cases where values are bound in a lexical scope
> > that's exited before the $sth->execute() call, so that's not safe.
> >
> > After the execute()? No, execute might be called again.
>
> If this is possible - docs/tests should reflect that (I am still fuzzy
> on what the semantics are, hence no patches to offer).
Does the example above clarify it?
> > After finish()? Maybe. The docs for finish is already say "Calling finish
> > ...
> > may also make some statement handle attributes (such as NAME and TYPE)
> > unavailable if they have not already been accessed (and thus cached)."
> > It seems reasonable to extend that to say that any references in
> > ParamValues will be weaked.
>
> After finish() won't work, because the app (DBIC in this case) doesn't
> know to call finish() before garbage collection, which never happens
> because of the circular ref. Augh!
> > Bound values are meant to persist across finish(), so there are still risks.
>
> Also undocumented ;)
Bound values are spec'd to persist in general. No need to mention finish().
> > I'm not keen on adding another attribute just to control this edge case.
>
> No, we shouldn't. After running through your thought process (thank you)
> it is clear this can not be fixed with conventional approaches. I need
> to wrap up Variable::Entangled that we specced out with Aristotle at
> last YAPC::EU but then neither of us got to completion.
>
> In the meantime stalling the bugreport with a link to this discussion
> sounds sane.
Will do.
Tim.
