Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date: 2021-07-06 07:58:03
Message-ID: YOQNCyfxp868zZUV@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 02, 2021 at 12:53:02PM +0000, kuroda(dot)hayato(at)fujitsu(dot)com wrote:
> I added such a message and some tests, but I began to think this is strange.
> Now I'm wondering why the connection is checked in some DESCRIPTOR-related
> statements? In my understanding connection name is not used in ECPGallocate_desc(),
> ECPGdeallocate_desc(), ECPGget_desc() and so on.
> Hence I think lookup_descriptor() and drop_descriptor() can be removed.
> This idea can solve your first argument.

I have been chewing on this comment and it took me some time to
understand what you meant here. It is true that the ecpglib part, aka
all the routines you are quoting above, don't rely at all on the
connection names. However, the preprocessor warnings generated by
drop_descriptor() and lookup_descriptor() seem useful to me to get
informed when doing incorrect descriptor manipulations, say on
descriptors that refer to incorrect object names. So I would argue
for keeping these.

0002 includes the following diffs:

-[NO_PID]: raising sqlcode -230 on line 111: invalid statement name "stmt_2" on line 111
-[NO_PID]: sqlca: code: -230, state: 26000
-SQL error: invalid statement name "stmt_2" on line 111
+[NO_PID]: deallocate_one on line 111: name stmt_2
+[NO_PID]: sqlca: code: 0, state: 00000
[...]
-[NO_PID]: raising sqlcode -230 on line 135: invalid statement name "stmt_3" on line 135
-[NO_PID]: sqlca: code: -230, state: 26000
-SQL error: invalid statement name "stmt_3" on line 135
+[NO_PID]: deallocate_one on line 135: name stmt_3
+[NO_PID]: sqlca: code: 0, state: 00000

And indeed, I would have expected those queries introduced by ad8305a
to pass. So a backpatch down to v14 looks adapted.

I am going to need more time to finish evaluating this patch, but it
seems that this moves to the right direction. The new warnings for
lookup_descriptor() and drop_descriptor() with the connection name are
useful. Should we have more cases with con2 in the new set of tests
for DESCRIBE?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2021-07-06 08:04:15 Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Previous Message Yugo NAGATA 2021-07-06 07:46:24 Re: [HACKERS] WIP aPatch: Pgbench Serialization and deadlock errors