Re: [PATCH] SQLFreeStmt deletes params, but does not reset the stmt->prepared state.

Hi Oliver,

On 2014/10/17 19:07, Oliver Freyd wrote:
> Hello,
>
> In ResolveOneParam (convert.c:4586) there is code that converts boolean
> '-1' to '1'. This is necessary for MS Access because it uses -1 as true
> representation.
>
> In my application, an Access 2013 frontend with a postgresql backend,
> sometimes this conversion did not work, and the backend would fail like
> this:
>
> invalid input syntax for type boolean: "-1" at character 399
>
> The statement is like this:
>
> DELETE FROM "public"."someview" WHERE "id" = 13020
> AND ... "someboolean" = '-1' ...
>
> This does not happen always, and I've seen it only in DELETE queries,
> when I try to delete a row that has a checkbox that is checked (true).
> The first try always works, the second try (deleting another row) it fails.
>
> Now this is how it fails (IMHO):
>
> The ResolveOneParam() function relies on the correct types being
> assigned to the parameters, In qb->ipdopts->parameters there is
> SQLType and PGType. The SQLType is and enum that does not contain
> boolean, only the PGType can be boolean.
>
> In PGAPI_Execute() (execute.c:1025) it calls prepareParameters, where
> the PGTypes are found (don't know how exactly), and put into the stmt,
> as stmt->ipd->ipdf.parameters.
>
> If the PGTypes are not 0, the query runs fine. The second time
> the query is executed, the query is already prepared (stmt->prepared=5)
> and the prepare step is omitted. But then the PGTypes are missing,
> the bool '-1' -> '1' conversion is not done and the query fails.
>
> It turns out Access calls SQLFreeStmt(option=SQL_RESET_PARAMS),
> that does SC_free_params, so the params get deleted.
>
> But the params contain the result of the prepare call,
> so IMHO the stmt->prepared state is now wrong, the transaction is no
> more prepared, so stmt->prepared should be reset to 0.
>
> And that actually fixes the bug.
>
>
> I hope this is useful, it took quite a while to track this down...

Thank you for your investigation.

According to the following pages
  http://msdn.microsoft.com/en-us/library/ms709284%28v=vs.85%29.aspx
  http://msdn.microsoft.com/en-us/library/ms710926%28v=vs.85%29.aspx

, it seems better to reset APD only keeping IPD intact.

Comments?

regards,
Hiroshi Inoue

> best regards,
>
>      Oliver Freyd
>
> -----------------------------------------------------------------------
> diff --git a/statement.c b/statement.c
> index da5abf5..a019d5d 100644
> --- a/statement.c
> +++ b/statement.c
> @@ -581,6 +581,7 @@ SC_free_params(StatementClass *self, char option)
>       {
>           APD_free_params(SC_get_APDF(self), option);
>           IPD_free_params(SC_get_IPDF(self), option);
> +        if (self->prepared!=NOT_YET_PREPARED)\
> SC_set_prepared(self, NOT_YET_PREPARED);
>       }
>       PDATA_free_params(SC_get_PDTI(self), option);
>       self->data_at_exec = -1;
>
>
>