Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105741
To: Robert Kopack <rkopack@tenable.com>, internals@lists.php.net
References: <CAMERU+4=XQ_8oji+McTjQB2pPXFLR1D6_2iWyFxiTVLCZX3pJQ@mail.gmail.com>
 <b7b52123-82ce-34bb-9ebf-365c0ac82ff3@gmx.de>
 <CAMERU+6vpSkDUBpgMtU0Cob=OGXcG98WvAX+MY9xzJHApovzMw@mail.gmail.com>
Message-ID: <42b836d2-5193-b2fd-bf0a-6ccf8843bc03@gmx.de>
Date: Sat, 18 May 2019 11:51:31 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <CAMERU+6vpSkDUBpgMtU0Cob=OGXcG98WvAX+MY9xzJHApovzMw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: de-DE
Content-Transfer-Encoding: quoted-printable
Subject: Re: RFC for Sqlite3 Extended Error Codes
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 16.05.2019 at 15:51, Robert Kopack wrote:

> On Wed, May 15, 2019 at 7:00 PM Christoph M. Becker <cmbecker69@gmx.de>
> wrote:
>
>> I haven't reviewed closely for now, but basically I like these
>> additions.  With regard to sqlite3_extended_result_codes() we have to
>> review existing error checks (they likely need small adjustments, if
>> extended error codes are enabled).  And we likely want to have a few
>> tests for the new functionality.
>
> I didn't consider that! And just to clarify are you referring to the
> results from sqlite3_extended_errcode() or the return value from
> sqlite3_extended_result_codes()?

I was referring to the slight behavioral change that is caused by
calling sqlite3_extended_result_codes(db, 1).  As I understand it, this
will cause other functions to possibly return extended error codes
instead of primary result codes.  It seems, though, that there would not
be any conflict right now, since only SQLITE_OK, SQLITE_ROW and
SQLITE_DONE are checked in ext/sqlite3, and these do not have extended
error codes.  Still it might make sense to mask these checks to prevent
future issues.  And of course, we would have to be careful, if we add
checks for e.g. SQLITE_BUSY in the future.

> If it's the latter, and after glancing at the sqlite source, it looks li=
ke
> that function returns either SQLITE_MISUSE (21) or SQLITE_OK (0). I put =
in
> -1 instead of 21 as the return for that function in error because I felt
> that was following what PHP normally does on error (perhaps FALSE would =
be
> better?) In fact, on further thought, returning the 0 from success on th=
at
> might be confusing as well. Perhaps it would better suited to check the
> return and return TRUE on SQLITE_OK and FALSE on SQLITE_MISUSE?

I think the method should return TRUE on success, and FALSE on failure.
An alternative might be to return the old value, like done by
SQLite3::enableExceptions().

> Thank you for your time!

Thanks for your's. :)

=2D-
Christoph M. Becker