Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date
Msg-id YOQNCyfxp868zZUV@paquier.xyz
Whole thread Raw
In response to RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE  ("kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>)
Responses Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
List pgsql-hackers
On Fri, Jul 02, 2021 at 12:53:02PM +0000, kuroda.hayato@fujitsu.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

Attachment

pgsql-hackers by date:

Previous
From: Yugo NAGATA
Date:
Subject: Re: [HACKERS] WIP aPatch: Pgbench Serialization and deadlock errors
Next
From: Michael Paquier
Date:
Subject: Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE