bucardo / dbdpg

Perl Postgres driver DBD::Pg aka dbdpg
48 stars 35 forks source link

ShowErrorStatement doesn't work on do("") requests #44

Closed dk closed 5 years ago

dk commented 5 years ago

Please compare the outputs of the two requests that should be identical, both with

$dbh->{ShowErrorStatement} = 1;

pre-executed, both sending a text to an integer argument:

my $sth = $dbh->prepare('select relname from pg_class where relpages = ?'); $sth->execute('foo');

DBD::Pg::st execute failed: ERROR: invalid input syntax for integer: "foo" [for Statement "select relname from pg_class where relpages = ?" with ParamValues: 1='foo']

and

$dbh->do('select relname from pg_class where id relpages ?', {}, 'bar');

DBD::Pg::db do failed: ERROR: syntax error at or near "relpages" LINE 1: select relname from pg_class where id relpages $1 ^ [for Statement "select relname from pg_class where id relpages ?"]

The latter is missing the ParamValues explained.

turnstep commented 5 years ago

At first glance, I don't know if this is possible. The problem is that DBI wants to call ParamValues on the current handle. For statement handles, this is no problem, as the placeholders are stored in the sth structure (which ParamValues walks through via dbd_st_FETCH_attrib). For database handles, however, we create a very local and temporary imp_sth inside of Pg.xs, which goes out of scope well before DBI asks $dbh for ParamValues (which it cannot handle anyway, as it knows nothing about it). We could in theory jump through some hoops to have imp_dbh start storing detailed information from its temporary imp_sth creation, but seems a lot of trouble for something so trivial.

I will expand the documentation to make this clearer, however.

dk commented 5 years ago

It would help debugging a lot to have parameters dumped together with the error. Or at the very least fix the documentation with example how to rewrite do() calls into something that respects ShowErrorStatement..

dk commented 5 years ago

I've sent a pull request please review. Please disregard d636fc6 , it's still there because only github didn't roll it back when I did 'git reset'.

turnstep commented 5 years ago

Added as of version 3.9.0