On 16.05.2019 at 15:51, Robert Kopack wrote:

> On Wed, May 15, 2019 at 7:00 PM Christoph M. Becker <cmbecke...@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 like
> 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 that
> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to