Re: Add function to release an allocated SQLDA

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: "Kato, Sho" <kato-sho(at)jp(dot)fujitsu(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add function to release an allocated SQLDA
Date: 2018-07-11 13:01:27
Message-ID: CAEepm=0O5sYhF1Oc0QDdKA7z29Cj0cBkBTDnFWnH_q2Caew8Sg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 19, 2018 at 9:11 PM, Kato, Sho <kato-sho(at)jp(dot)fujitsu(dot)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 one over 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-07-11 13:13:40 Re: _isnan() on Windows
Previous Message Heikki Linnakangas 2018-07-11 13:00:47 Re: Negotiating the SCRAM channel binding type