On Mon, 9 Sep 2002, Tim Bunce wrote:
> > Here is a patch againt DBI-1.30 which adds some important
> > functionality and fixes a minor problem or two.
>
> Ohhh, thanks. This is good stuff. I've a few assorted comments and
> suggested tweaks...
>
> > A bug in the ProxyServer table_info() method was fixed. With Perl
> > 5.8.0 and DBD::DB2 driver v1.26, calling $sth->fetch() in scalar
> > context is a losing proposition, resulting in the dreaded "Can't use
> > scalar '1' as an array ref.." exception.
>
> Why? A DBD::DB2 driver bug?
I'm not sure. Wouldn't any fetch() method returning an array behave in
the same manner? For whatever reason, we did not see this problem until
Perl 5.8.0, so I'm not convinced it's a problem with the DB2 driver.
> > I'm not sure if anyone has ever used 'dbish' to work with a proxied
> > database by setting DBI_AUTOPROXY, but as it stands the DBD::Sponge
> > pseudo driver gets invoked on the far end unless measures are taken.
> > This _should_ work, I suppose, but the dbish 'table_info' command
> > causes a dramatic core dump on the client side when run in this
> > manner. I suspect a bug in Storable, but have not been motivated to
> > track it down. Should be repeatable if anyone else wants to pursue
> > it. Forcibly clearing the environment variable after the initial
> > connection is setup ensures that Sponge runs on the client. Life is
> > then Good.
>
> I think a better fix would be to change this line in DBI::connect()
> to include 'Sponge':
> if ($ENV{DBI_AUTOPROXY} && $driver ne 'Proxy' && $driver ne 'Switch') {
Looks right. I'll put it in.
> > The dbish semantics for table_info also require that a Proxy
> > table_info call set the statement handle 'Active'. This now works
> > properly.
>
> Thanks. (Though I'm not clear on which bit of the patch addresses that.)
This line in Proxy.pm table_info():
$sth->SUPER::STORE('Active' => 1) if @rows;
> > Synchronous prepare / execute:
> >
> > I remain unconvinced that the original "lazy" method (deferring
> > prepare until the first call to execute) was truly saving any time.
>
> For applications not using placeholders and doing lots of non-selects
> it can significantly reduce the (expensive) round-trips to the proxy server.
>
> > Further, although the DBI spec may legalize deferred prepare,
> > enforcing this on the proxy session when the underlying remote
> > database has meaningful and distinct prepare behavior violates the
> > principle of least surprise.
>
> Adding support for non-lazy prepare is a valuable addition - but
> it needs to be made configurable at the dbh level via an attribute.
I'm not sure I agree with you on this. The Proxy should remain as
transparent as possible to any client applications. By treating it as a
another attribute in the DBI_AUTOPROXY dsn string, we can avoid the need
to make any changes to the application. Unless I'm misunderstanding you,
there would be no way to avoid this if it's only available through the
dbh.
How about if we just default to the original protocol version?
> > It is often desirable to suppress DBI exceptions and have the program
> > logic deal with common (or expected) cases. For this to work
> > transparently, the application must see the actual error information
> > generated by the remote database. My proposal is to always turn off
> > 'RaiseError' on the ProxyServer and pass back the database-specific
> > codes on failure.
>
> The code as patched treats execute() returning 0 as an error.
> It's should check for undef instead.
I had it that way originally, but changed it in the (probably erroneous)
belief that fetch() returned "OEO" (aka "zero but true") for a successful
call returning a value of 0. Nonetheless, you are quite correct.
> But I don't see why execute() should be made into a special case.
> I'd much rather see RaiseError always used on the server (for
> simplicity, consistency, and reliability) and have the code
> fixed to pass back err & errstr & state properly.
>
> The basic pattern, oft repeated, in the DBI::Proxy code is:
>
> ... = eval { $remote_h->foo(...) };
> return DBI::set_err($local_h, 1, $@) if $@;
>
> which is just plain insufficient and needs fixing. The easiest
> approach I can see is probably to use a HandleError => sub { ... }
> in the server to edit the error message so it contains the err and
> state values in a way that can be reliably extracted later, but still
> human readable and compatible with ShowErrorStatement and ParamValues etc.
> A bit of a hack but very workable. Something roughly like:
>
> HandleError => sub {
> my $err = $_[1]->err;
> my $state = $_[1]->state || '';
> $_[0] .= " [err=$err,state=$state]";
> return 0;
> }
>
> the DBI::Proxy mantra could be changed to abtract out the handling into a sub:
>
> ... = eval { $remote_h->foo(...) };
> return DBD::Proxy::proxy_set_err($local_h, $@) if $@;
>
> sub DBD::Proxy::proxy_set_err {
> my ($h, $errmsg) = @_;
> my ($err, $state) = ($errmsg =~ s/ \[err=(.*?),state=(.*?)\]//) ? ($1,$2)
>: (1,'');
> return DBI::set_err($h, $err, $errmsg, $state);
> }
>
> That would be backwards compatible and would fix *every* remote call
> to pass back err & errstr & state properly.
Cool. I'll play with that a bit.
> I'm very glad to see the proxy code getting more use and I'd love
> to see it improved. Thanks for helping out.
You are quite welcome. Thank YOU for DBI in general! It's made
development of database applications fun.
Steve
--
----------------------------------------------------------------
Steven N. Hirsch tie-line: 446-6557 ext: 802-769-6557
Staff Engineer Methodology Integration Team
ASIC Product Development IBM Microelectronics
----------------------------------------------------------------
