Thread: Add function to release an allocated SQLDA
Hi,
I add a function called ECPGfreeSQLDA() becasue there is no API for releasing the SQLDA stored the result set.
An example of usage is as follows.
Specify a pointer to sqlda_t to be released as an argument.
Example:
exec sql begin declare section;
char *stmt1 = "SELECT * FROM t1";
exec sql end declare section;
sqlda_t *outp_sqlda;
exec sql prepare st_id2 from :stmt1;
exec sql declare mycur2 cursor for st_id1;
exec sql open mycur2;
exec sql fetch all from mycur2 into descriptor outp_sqlda;
exec sql close mycur2;
exec sql deallocate prepare st_id2;
ECPGfreeSQLDA(outp_sqlda);
The patch is attached.
The threads involved in this patch are as follows.
https://www.postgresql.org/message-id/25C1C6B2E7BE044889E4FE8643A58BA963A42097@G01JPEXMBKW03
https://www.postgresql.org/message-id/flat/0A3221C70F24FB45833433255569204D1F8AD5D6@G01JPEXMBYT05#0A3221C70F24FB45833433255569204D1F8AD5D6@G01JPEXMBYT05
regards,
--
Kato Sho
Attachment
On Wed, Jun 13, 2018 at 4:29 PM, Kato, Sho <kato-sho@jp.fujitsu.com> wrote: > I add a function called ECPGfreeSQLDA() becasue there is no API for releasing the SQLDA stored the result set. Hello Kato-san, Thank you for sending the patch! + Alternatively, use the standard C library's free() function as in the example below. + If the result set contains more than one record, an SQLDA corresponding to each records is saved as linked list. + There is a possibility to free allocated memory area doubly and cause the application crash, + because ECPGfreeSQLDA() releases all SQLDAs associated with the specified the SQLDA. This is not clear to me. ECPGfreeSQLDA() releases a whole chain, but free() only releases a single SQLDA(), so they are obviously not interchangeable. When exactly should a user prefer one over the other? If there is a good reason to free just one OR the whole chain, shouldn't we provide a way to do both of those things without the user having to use libc free(), for the benefit of Windows users who can't safely use free()? - * the new partition's info into its partition descriptor. If there is a + * the new partition's info into its partition descriptor. If there is a This seems to be a spurious hunk, but I cannot see what changed. -- Thomas Munro http://www.enterprisedb.com
Hi Thomas Thank you for your reply. >This is not clear to me. ECPGfreeSQLDA() releases a whole chain, but >free() only releases a single SQLDA(), so they are obviously not interchangeable. When exactly should a user prefer oneover the other? If an application use FETCH ALL to get the result set at once, a user should use ECPGfreeSQLDA(). Because the user does not know that the SQLDA holds the result set in the chain. If it does not release it will remain leaked. Considering the use of people who know the structure of SQLDA, I described the releasing method using free () in the document. >If there is a good reason to free just one OR the whole chain, shouldn't we provide a way to do both of those things withoutthe user having to use libc free(), for the benefit of Windows users who can't safely use free()? I think so. Ok, The behavior when releasing one SQLDA with ECPGfreeSQLDA() is the same as free() and using free() is not safety for Windowsusers. So, how about trying to delete the release method using free ()? >This seems to be a spurious hunk, but I cannot see what changed. Sorry, this is a mistake.. -----Original Message----- From: Thomas Munro [mailto:thomas.munro@enterprisedb.com] Sent: Monday, June 18, 2018 2:42 PM To: Kato, Sho/加藤 翔 <kato-sho@jp.fujitsu.com> Cc: Pg Hackers <pgsql-hackers@postgresql.org> Subject: Re: Add function to release an allocated SQLDA On Wed, Jun 13, 2018 at 4:29 PM, Kato, Sho <kato-sho@jp.fujitsu.com> wrote: > I add a function called ECPGfreeSQLDA() becasue there is no API for releasing the SQLDA stored the result set. Hello Kato-san, Thank you for sending the patch! + Alternatively, use the standard C library's free() function as in the example below. + If the result set contains more than one record, an SQLDA corresponding to each records is saved as linked list. + There is a possibility to free allocated memory area doubly and cause the application crash, + because ECPGfreeSQLDA() releases all SQLDAs associated with the specified the SQLDA. This is not clear to me. ECPGfreeSQLDA() releases a whole chain, but free() only releases a single SQLDA(), so they are obviously not interchangeable. When exactly should a user prefer oneover the other? If there is a good reason to free just one OR the whole chain, shouldn't we provide a way to do bothof those things without the user having to use libc free(), for the benefit of Windows users who can't safely use free()? - * the new partition's info into its partition descriptor. If there is a + * the new partition's info into its partition descriptor. If there is + a This seems to be a spurious hunk, but I cannot see what changed. -- Thomas Munro http://www.enterprisedb.com
On Tue, Jun 19, 2018 at 9:11 PM, Kato, Sho <kato-sho@jp.fujitsu.com> wrote:
>>This is not clear to me. ECPGfreeSQLDA() releases a whole chain, but
>>free() only releases a single SQLDA(), so they are obviously not interchangeable. When exactly should a user prefer
oneover the other?
>
> If an application use FETCH ALL to get the result set at once, a user should use ECPGfreeSQLDA().
> Because the user does not know that the SQLDA holds the result set in the chain.
> If it does not release it will remain leaked.
>
> Considering the use of people who know the structure of SQLDA, I described the releasing method using free () in the
document.
I wonder how other ESQL/C implementations deal with this stuff. The
DB2 SQLDA struct[1] doesn't have desc_next. The Informix one[2] does,
but I'm not entirely sure what it's for since Informix apparently
doesn't support EXEC SQL FETCH ALL. The Informix manual also lists a
function SqlFreeMem() that is needed on Windows for the same reason we
need a new function[3]. I don't think there is anyone left who cares
about compatibility with Informix so (unless there is some guidance
from the spec here) I think your proposed name ECPGfreeSQLDA() is OK.
test/sql/sqlda.pgc includes a bit where SQLDA objects are still freed
with free(). That's because the loop does extra processing with each
SQLA:
while (outp_sqlda1)
{
....blah blah...
outp_sqlda1 = outp_sqlda1->desc_next;
free(ptr);
}
To allow that type of usage, we would need two new functions:
ECPGfreeSQLDA() to free just one, and ECPGfreeAllSQLDA() to free the
whole chain. The minimum thing to back-patch would be a
EDPGfreeSQLDA() function to free just one, since that fixes a
potential bug for Window users (the same reason we back-patched
4c8156d87108fa1f245bee13775e76819cd46a90). Then ECPGfreeAllSQLDA()
(or better name) could be a separate patch to discuss for PG 12. Does
that make sense?
sqlda.c:
+void
+ECPGfreeSQLDA_informix(struct sqlda_compat *sqlda_ptr)
+{
ecpglib.h:
+void ECPGfreeSQLDA_compat(struct sqlda_compat *);
I think these function names were supposed to match?
[1] https://www.ibm.com/support/knowledgecenter/en/SSEPGG_9.7.0/com.ibm.db2.luw.apdv.api.doc/doc/r0001751.html
[2] https://www.ibm.com/support/knowledgecenter/en/SSGU8G_12.1.0/com.ibm.esqlc.doc/ids_esqlc_0656.htm
[3] https://www.ibm.com/support/knowledgecenter/en/SSGU8G_11.50.0/com.ibm.esqlc.doc/xbsql1007134.htm
--
Thomas Munro
http://www.enterprisedb.com
> On Wed, 13 Jun 2018 at 06:30, Kato, Sho <kato-sho@jp.fujitsu.com> wrote: > > I add a function called ECPGfreeSQLDA() becasue there is no API for releasing the SQLDA stored the result set. > > On Mon, 18 Jun 2018 at 07:42, Thomas Munro <thomas.munro@enterprisedb.com> wrote: > > I wonder how other ESQL/C implementations deal with this stuff (...) > To allow that type of usage, we would need two new functions: (...) Thank you for the patch. Unfortunately, judging from the cfbot.cputube.org it can't be applied anymore to the current master. Could you please rebase the code and address the reviewer feedback?
> On Wed, Nov 14, 2018 at 6:01 PM Dmitry Dolgov <9erthalion6@gmail.com> wrote: > > > On Wed, 13 Jun 2018 at 06:30, Kato, Sho <kato-sho@jp.fujitsu.com> wrote: > > > > I add a function called ECPGfreeSQLDA() becasue there is no API for releasing the SQLDA stored the result set. > > > > On Mon, 18 Jun 2018 at 07:42, Thomas Munro <thomas.munro@enterprisedb.com> wrote: > > > > I wonder how other ESQL/C implementations deal with this stuff (...) > > To allow that type of usage, we would need two new functions: (...) > > Thank you for the patch. Unfortunately, judging from the cfbot.cputube.org it > can't be applied anymore to the current master. Could you please rebase the > code and address the reviewer feedback? Due to lack of response I'm marking this as returned with feedback.