shell.c: exec_prepared_stmt no return value

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

shell.c: exec_prepared_stmt no return value

Hannes Mühleisen
Hello SQLite list,

we have noticed that the sqlite shell is unable to report errors that happen within exec_prepared_stmt, because that function has no return value and is thus unable to bubble issues up. For example, if sqlite3_step should fail for some reason, this should be shown to the user, for example in the call to exec_prepared_stmt from shell_exec.

We propose to add a return code to exec_prepared_stmt like so:

static int exec_prepared_stmt(
 ShellState *pArg,                                /* Pointer to ShellState */
 sqlite3_stmt *pStmt                              /* Statment to run */
){
 int rc;
        /* ... */
return rc;
}

then, in shell_exec, we could say something like

 rc = exec_prepared_stmt(pArg, pStmt);
                if (rc != SQLITE_OK) {
                        if (pzErrMsg) {
                                *pzErrMsg = save_err_msg(db);
                        }
                }

This way, if an error occurs during execution, this will be displayed to the user.

Best from Amsterdam,

Hannes

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: shell.c: exec_prepared_stmt no return value

Richard Hipp-3
Thank you for the bug report.

However, you have provided a fix without showing us the malfunction.
You suggest a change without demonstrating what behavior the change is
designed to fix.  The problem you are trying to fix is not obvious,
because when I run test queries that contain errors, I do get an error
message back, even without your fix.  So I cannot figure out what
problem your fix is intending to address.

Can you please provide an example input that gives incorrect results
before your proposed fix, and that gives the correct result
afterwards?

On 1/8/20, Hannes Mühleisen <[hidden email]> wrote:

> Hello SQLite list,
>
> we have noticed that the sqlite shell is unable to report errors that happen
> within exec_prepared_stmt, because that function has no return value and is
> thus unable to bubble issues up. For example, if sqlite3_step should fail
> for some reason, this should be shown to the user, for example in the call
> to exec_prepared_stmt from shell_exec.
>
> We propose to add a return code to exec_prepared_stmt like so:
>
> static int exec_prepared_stmt(
>  ShellState *pArg,                                /* Pointer to ShellState
> */
>  sqlite3_stmt *pStmt                              /* Statment to run */
> ){
>  int rc;
> /* ... */
> return rc;
> }
>
> then, in shell_exec, we could say something like
>
>  rc = exec_prepared_stmt(pArg, pStmt);
> if (rc != SQLITE_OK) {
> if (pzErrMsg) {
> *pzErrMsg = save_err_msg(db);
> }
> }
>
> This way, if an error occurs during execution, this will be displayed to the
> user.
>
> Best from Amsterdam,
>
> Hannes
>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>


--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: shell.c: exec_prepared_stmt no return value

Hannes Mühleisen
Thanks for your quick response. Turns out while trying to provoke the issue in a test case I realised a misunderstanding the SQLite API on my part: errors during execution are bubbled up to the shell by sqlite3_finalize.

My apologies,

Hannes


> On 8 Jan 2020, at 13:30, Richard Hipp <[hidden email]> wrote:
>
> Thank you for the bug report.
>
> However, you have provided a fix without showing us the malfunction.
> You suggest a change without demonstrating what behavior the change is
> designed to fix.  The problem you are trying to fix is not obvious,
> because when I run test queries that contain errors, I do get an error
> message back, even without your fix.  So I cannot figure out what
> problem your fix is intending to address.
>
> Can you please provide an example input that gives incorrect results
> before your proposed fix, and that gives the correct result
> afterwards?
>
> On 1/8/20, Hannes Mühleisen <[hidden email]> wrote:
>> Hello SQLite list,
>>
>> we have noticed that the sqlite shell is unable to report errors that happen
>> within exec_prepared_stmt, because that function has no return value and is
>> thus unable to bubble issues up. For example, if sqlite3_step should fail
>> for some reason, this should be shown to the user, for example in the call
>> to exec_prepared_stmt from shell_exec.
>>
>> We propose to add a return code to exec_prepared_stmt like so:
>>
>> static int exec_prepared_stmt(
>> ShellState *pArg,                                /* Pointer to ShellState
>> */
>> sqlite3_stmt *pStmt                              /* Statment to run */
>> ){
>> int rc;
>> /* ... */
>> return rc;
>> }
>>
>> then, in shell_exec, we could say something like
>>
>> rc = exec_prepared_stmt(pArg, pStmt);
>> if (rc != SQLITE_OK) {
>> if (pzErrMsg) {
>> *pzErrMsg = save_err_msg(db);
>> }
>> }
>>
>> This way, if an error occurs during execution, this will be displayed to the
>> user.
>>
>> Best from Amsterdam,
>>
>> Hannes
>>
>> _______________________________________________
>> sqlite-users mailing list
>> [hidden email]
>> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
>>
>
>
> --
> D. Richard Hipp
> [hidden email]

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users