RaiseWarn attribute for DBI

[pa...@cpan.org]

Hello!

What do you think about adding a new attribute $dbh->{RaiseWarn} which
cause that warnings reported by DBI drivers would behave like errors?

For errors DBI has there $dbh->{PrintError} and $dbh->{RaiseError}
attributes. First one is by default true and second one by default
false. When PrintWarn is true, then all error from DBI driver are passed
to perl's "warn" function and when RaiseError is true, then errors are
passed to perl's "die" function. (Plus there is ability to register own
error handler function)

Currently DBI has only $dbh->{PrintWarn} attribute to control warnings.
When is set to true (by default) all warnings from DBI driver are passed
to perl's "warn" function.

So I would propose to add $dbh->{RaiseWarn} attribute (off by default)
to behave like $dbh->{RaiseError}, but for warnings.

I have implemented this attribute and patch is there:
https://github.com/perl5-dbi/dbi/pull/71/files

[Alexander Hartmaier]

I don't see the benefit, Print* should die and I'd personally would release a major release and change the defaults as a breaking change: PrintError false, RaiseError true.

Can you name a use case and how to differ between an error and a warning at the error handling side?

 

Best regards, Alex

[pa...@cpan.org]

On Thursday 17 January 2019 10:23:25 Alexander Hartmaier wrote:
> I don't see the benefit, Print* should die

This would break existing API of DBI. Print just prints and Raise dies.
This cannot be changed as there are many applications which depends on
this API.

> and I'd personally would release a major release and change the defaults as a breaking change: PrintError false, RaiseError true.

> Can you name a use case and how to differ between an error and a warning at the error handling side?

It is up to the DBI driver or database server what would result in is a
warning and what in an error.

[Alexander Hartmaier]

I'm aware of that, semantic versioning and major versions exist to handle API breakage.

 

My question is how to detect if an exception is because of a warn or a die when RaiseWarn is true.

 

Best regards, Alex

[pa...@cpan.org]

On Thursday 17 January 2019 11:06:22 Alexander Hartmaier wrote:
> I'm aware of that, semantic versioning and major versions exist to handle API breakage.

Such thing is unsupported by CPAN clients. So we cannot use it.

Anyway, this is question for Tim as DBI maintainer. But I guess he does
not want to change API of DBI.

> My question is how to detect if an exception is because of a warn or a die when RaiseWarn is true.

I guess you can use $dbh->err() method.
See: https://metacpan.org/pod/DBI#err

A driver may return 0 from err() to indicate a warning condition after a
method call. Similarly, a driver may return an empty string to indicate
a 'success with information' condition. In both these cases the value is
false but not undef.

Note that 'success with information' is not warning and therefore DBI's
PrintWarn or RaiseWarn ignores them.

[Darren Duncan]

Generally speaking, DBI is one of those things where backwards compatibility
should be the highest concern after security. There is a time and place to
break compatibility, and this Print/Raise thing seems way too minor to me for
that. I support the version of this that is backwards-compatible and not the
breaking version. -- Darren Duncan

[pa...@cpan.org]

Personally I do not like changing Print/Raise. It is documented,
implementation seems to match documentation, it is without bugs and
current behavior is usable.

Anyway, back to my question about RaiseWarn. Do you think that it make
sense to have it in DBI?

[Alexander Hartmaier]

I didn‘t want to start a discussion about deprecation because I know the opinion about that for most Perl 5 developers.

But strictures and its use in Moo showed that exceptions from warnings aren‘t welcome.
You can install a warn handler in your code without requiring any change. Adding another option raises the number of combinations.

As I don‘t see any benefit and no examples where given I‘m against it.

[Tim Bunce]

On Thu, Jan 17, 2019 at 10:02:39AM +0100, pa...@cpan.org wrote:
>
> Currently DBI has only $dbh->{PrintWarn} attribute to control warnings.
> When is set to true (by default) all warnings from DBI driver are passed
> to perl's "warn" function.
>
> So I would propose to add $dbh->{RaiseWarn} attribute (off by default)
> to behave like $dbh->{RaiseError}, but for warnings.

I'd like to know more about the specific use-case(s) that prompted this.

Tim.

p.s. Re the compatibility topic in this thread: there will be no
breaking changes to DBI.

[pa...@cpan.org]

On Friday 18 January 2019 09:13:48 Tim Bunce wrote:
> On Thu, Jan 17, 2019 at 10:02:39AM +0100, pa...@cpan.org wrote:
> >
> > Currently DBI has only $dbh->{PrintWarn} attribute to control warnings.
> > When is set to true (by default) all warnings from DBI driver are passed
> > to perl's "warn" function.
> >
> > So I would propose to add $dbh->{RaiseWarn} attribute (off by default)
> > to behave like $dbh->{RaiseError}, but for warnings.
>
> I'd like to know more about the specific use-case(s) that prompted this.

Hi! The use-case is for testing code, that its SQL part does not produce
any warning. Lot of database server supports vendor specific SQL command
to convert warnings to errors, but there is no standard way how to do it
driver/database independent. And because DBI reports warnings via Perl's
warn, it is not possible to easily distinguish between DBI warnings,
internal Perl warnings and warnings generated by other 3rd modules.

So use-case is: raise DBI errors for any warning or error from database
server and let warnings reported by 3rd modules and by Perl itself as
is. So to ensure that database server does not produce any "problems".

[Tim Bunce]

Couldn't HandleSetErr be used for this?

Quoting the docs at https://metacpan.org/pod/DBI#HandleSetErr

HandleSetErr, on the other hand, is called whenever set_err() is
called with a defined err value, even if false. So it's not just for
errors, despite the name, but also warn and info states. The set_err()
method, and thus HandleSetErr, may be called multiple times within a
method and is usually invoked from deep within driver code.

Tim.

[pa...@cpan.org]

I'm looking at it and will try some tests...

Anyway, another use-case is for testing DBI SQL applications. If I write
tests for that application which should not produce any SQL warnings,
then with my approach RaiseWarn => 1 in DBI->connect I can simple ensure
that test fails if something unexpected happens.

With HandleSetErr it is maybe possible too, but needs non-trivial logic
for writing code ref subroutine and is not so simple and obvious for
people who read test code. With RaiseWarn => 1 I simple declare what
should happen when warning is generated; similarly like for already
existing RaiseError.

Reason why I chose RaiseWarn is because there is already PrintWarn,
PrintError and RaiseError attributes. So RaiseWarn just use same naming
and logic pattern.

[pa...@cpan.org]

Hi Tim! Seems that HandleSetErr cannot be used. It is not called for
cases which generates warnings.

On the other hand my proposed and implemented RaiseWarn is working fine.

[Tim Bunce]

On Fri, Apr 12, 2019 at 10:22:57AM +0200, pa...@cpan.org wrote:
> On Monday 21 January 2019 16:55:07 pa...@cpan.org wrote:
> > On Sunday 20 January 2019 17:27:22 Tim Bunce wrote:
> > >
> > > Couldn't HandleSetErr be used for this?
> > >
> > > Quoting the docs at https://metacpan.org/pod/DBI#HandleSetErr
> > >
> > > HandleSetErr, on the other hand, is called whenever set_err() is
> > > called with a defined err value, even if false. So it's not just for
> > > errors, despite the name, but also warn and info states. The set_err()
> > > method, and thus HandleSetErr, may be called multiple times within a
> > > method and is usually invoked from deep within driver code.

> Hi Tim! Seems that HandleSetErr cannot be used. It is not called for
> cases which generates warnings.

Couldn't that be fixed?

Tim.

[pa...@cpan.org]

On Monday 15 April 2019 16:21:30 Tim Bunce wrote:
> On Fri, Apr 12, 2019 at 10:22:57AM +0200, pa...@cpan.org wrote:
> > On Monday 21 January 2019 16:55:07 pa...@cpan.org wrote:
> > > On Sunday 20 January 2019 17:27:22 Tim Bunce wrote:
> > > >
> > > > Couldn't HandleSetErr be used for this?
> > > >
> > > > Quoting the docs at https://metacpan.org/pod/DBI#HandleSetErr
> > > >
> > > > HandleSetErr, on the other hand, is called whenever set_err() is
> > > > called with a defined err value, even if false. So it's not just for
> > > > errors, despite the name, but also warn and info states. The set_err()
> > > > method, and thus HandleSetErr, may be called multiple times within a
> > > > method and is usually invoked from deep within driver code.
>
> > Hi Tim! Seems that HandleSetErr cannot be used. It is not called for
> > cases which generates warnings.
>
> Couldn't that be fixed?

I do not know. Code around HandleSetErr seems to be complex.

I still think that when there is already PrintError, RaiseError and
PrintWarn attributes, that one RaiseWarn is missing to be API complete.