Обсуждение: Re: [PATCH] SQLFreeStmt deletes params, but does not reset the stmt->prepared state.
Re: [PATCH] SQLFreeStmt deletes params, but does not reset the stmt->prepared state.
От
"Faith, Jeremy"
Дата:
Hi,
I also found this issue with the PHP odbc extension which uses SQL_RESET_PARAMS.
It causes the following error on the second odbc_execute() call.
PHP Warning: odbc_execute(): SQL error: Unfortunatley couldn't get this
paramater's info, SQL state S1000 in SQLDescribeParameter
>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?
I tried Oliver's patch and PHP no longer has a problem.
Inserting 100,000 rows into a simple table takes 47.6s
No sign of any memory leak.
On the other hand commenting out the IPD_free_params() call also works.
Same 100,000 inserts take 34.1s, so significantly faster.
Again no sign of a memory leak. I also tried prepare,execute,free 100K times, much slower but again no sign of a memory leak.
I think the SC_Destructor() function will free the IPD params if nothing else does. So, unless there is some other path that could cause a leak, I would say just commenting out the IPD_free_params() call is a better solution.
Regards,
Jeremy Faith
>
>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;
>>
>>
>>
Tyco Safety Products/CEM Systems Ltd.I also found this issue with the PHP odbc extension which uses SQL_RESET_PARAMS.
It causes the following error on the second odbc_execute() call.
PHP Warning: odbc_execute(): SQL error: Unfortunatley couldn't get this
paramater's info, SQL state S1000 in SQLDescribeParameter
>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?
I tried Oliver's patch and PHP no longer has a problem.
Inserting 100,000 rows into a simple table takes 47.6s
No sign of any memory leak.
On the other hand commenting out the IPD_free_params() call also works.
Same 100,000 inserts take 34.1s, so significantly faster.
Again no sign of a memory leak. I also tried prepare,execute,free 100K times, much slower but again no sign of a memory leak.
I think the SC_Destructor() function will free the IPD params if nothing else does. So, unless there is some other path that could cause a leak, I would say just commenting out the IPD_free_params() call is a better solution.
Regards,
Jeremy Faith
>
>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;
>>
>>
>>
This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.
Re: Re: [PATCH] SQLFreeStmt deletes params, but does not reset the stmt->prepared state.
От
Oliver Freyd
Дата:
Hi, I think I'm using the psql-odbc driver hand-patched with IPD_free_params() commented out, like you said, and it works fine for us. Isnt' this already merged into the git repository? I thought it sounded pretty obvious at the time when I found it, didn't bother to check afterwards... greets, Oliver Am 24.02.2015 um 16:03 schrieb Faith, Jeremy: > Hi, > > I also found this issue with the PHP odbc extension which uses > SQL_RESET_PARAMS. > It causes the following error on the second odbc_execute() call. > PHP Warning: odbc_execute(): SQL error: Unfortunatley couldn't get this > paramater's info, SQL state S1000 in SQLDescribeParameter > > > >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? > > > I tried Oliver's patch and PHP no longer has a problem. > Inserting 100,000 rows into a simple table takes 47.6s > No sign of any memory leak. > > On the other hand commenting out the IPD_free_params() call also works. > Same 100,000 inserts take 34.1s, so significantly faster. > Again no sign of a memory leak. I also tried prepare,execute,free 100K > times, much slower but again no sign of a memory leak. > > I think the SC_Destructor() function will free the IPD params if nothing > else does. So, unless there is some other path that could cause a leak, > I would say just commenting out the IPD_free_params() call is a better > solution. > > Regards, > Jeremy Faith > > > > >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; > >> > >> > >> > Tyco Safety Products/CEM Systems Ltd. > ------------------------------------------------------------------------ > > This e-mail contains privileged and confidential information intended > for the use of the addressees named above. If you are not the intended > recipient of this e-mail, you are hereby notified that you must not > disseminate, copy or take any action in respect of any information > contained in it. If you have received this e-mail in error, please > notify the sender immediately by e-mail and immediately destroy this > e-mail and its attachments.
Re: Re: [PATCH] SQLFreeStmt deletes params, but does not reset the stmt->prepared state.
От
"Faith, Jeremy"
Дата:
On 25 February 2015 15:27, Oliver Freyd wrote: >Hi, > >I think I'm using the psql-odbc driver hand-patched with >IPD_free_params() commented out, like you said, and it works >fine for us. > >Isnt' this already merged into the git repository? >I thought it sounded pretty obvious at the time when I found it, >didn't bother to check afterwards... > >greets, >Oliver Hi, I checked out the latest from git and the problem still exists. Regards, Jeremy Faith > > >Am 24.02.2015 um 16:03 schrieb Faith, Jeremy: >> Hi, >> >> I also found this issue with the PHP odbc extension which uses >> SQL_RESET_PARAMS. >> It causes the following error on the second odbc_execute() call. >> PHP Warning: odbc_execute(): SQL error: Unfortunatley couldn't get this >> paramater's info, SQL state S1000 in SQLDescribeParameter >> >> >> >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? >> >> >> I tried Oliver's patch and PHP no longer has a problem. >> Inserting 100,000 rows into a simple table takes 47.6s >> No sign of any memory leak. >> >> On the other hand commenting out the IPD_free_params() call also works. >> Same 100,000 inserts take 34.1s, so significantly faster. >> Again no sign of a memory leak. I also tried prepare,execute,free 100K >> times, much slower but again no sign of a memory leak. >> >> I think the SC_Destructor() function will free the IPD params if nothing >> else does. So, unless there is some other path that could cause a leak, >> I would say just commenting out the IPD_free_params() call is a better >> solution. >> >> Regards, >> Jeremy Faith >> >> > >> >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; >> >> >> >> >> >> Tyco Safety Products/CEM Systems Ltd. ________________________________ This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you arenot the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any actionin respect of any information contained in it. If you have received this e-mail in error, please notify the senderimmediately by e-mail and immediately destroy this e-mail and its attachments.