Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105977
MIME-Version: 1.0
References: <CAMERU+4=XQ_8oji+McTjQB2pPXFLR1D6_2iWyFxiTVLCZX3pJQ@mail.gmail.com>
 <b7b52123-82ce-34bb-9ebf-365c0ac82ff3@gmx.de> <CAMERU+6vpSkDUBpgMtU0Cob=OGXcG98WvAX+MY9xzJHApovzMw@mail.gmail.com>
 <42b836d2-5193-b2fd-bf0a-6ccf8843bc03@gmx.de>
In-Reply-To: <42b836d2-5193-b2fd-bf0a-6ccf8843bc03@gmx.de>
Date: Tue, 18 Jun 2019 15:34:41 -0400
Message-ID: <CAMERU+7ARqRDNXtQE3Zwp7-37uiBekZzFXdzKYGZ98y-2L7h=A@mail.gmail.com>
To: internals@lists.php.net
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: RFC for Sqlite3 Extended Error Codes
From: rkopack@tenable.com (Robert Kopack)

Hello everybody,

I've had the PR open for this a while now
(https://github.com/php/php-src/pull/4166) and was hoping to get more
eyes on it; Kalle was looking at it but he appears to have not been as
active lately and as I believe it is in a good state right now I was
hoping to have some other feedback on it. I was also curious if this
would go into a 7.x version of PHP or if something like this would
only be for the next major version (8.0).

Thank you,
Robert

On Sat, May 18, 2019 at 5:51 AM Christoph M. Becker <cmbecker69@gmx.de> wro=
te:
>
> 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 l=
ike
> > 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 fel=
t
> > 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 t=
hat
> > 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. :)
>
> --
> Christoph M. Becker
>