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

At Fri, 25 Jun 2021 12:02:22 +0000, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> wrote in 
> Dear Hackers,
> 
> I checked about DECLARE STATEMENT(added from ad8305a), and I noticed that
> this connection-control feature cannot be used for DEALLOCATE and DESCRIBE statement.
> 
> I attached the patch that fixes these bugs, this contains source and test code.
> 
> How do you think? 

(Maybe by consulting the code.. Anyway, )

prepared_name is used in the follwoing statement rules.

The following commands handle the liked connection.
DECLARE
PREPARE
EXECUTE

The follwoing commands don't.
DESCRIBE
DEALLOCATE
DECLARE CURSOR .. FOR
CREATE TABLE AS EXECUTE

Although I'm not sure it is definitely a bug or not, it seems
reasonable that the first two follow the liked connection.

I'm not sure about the last two. Since ecpg doesn't allow two prepared
statements with the same name even if they are on different
connections.  So the two can also follow the connection linked to the
given statements. DECLARE CURSOR could be follow the liked connection
safely but CREATE TABLE AS EXECUTE doesn't seem safe.

I'm not sure how ALLOCATE DESCRIPTOR should behave. Without "AT conn"
attached, the descriptor is recorded being bound to the connection
"null"(or nothing). GET DESCRIPTOR for the statement stmt tries to
find a descriptor with the same name but bound to c1, which does not
exist.

As the result ecpg complains like this:

    EXEC SQL CONNECT TO 'db1@,..' AS c1;
    EXEC SQL AT c1 DECLARE stmt STATEMENT;
    EXEC SQL PREPARE stmt FROM "...";
    EXEC SQL ALLOCATE DESCRIPTOR desc;
    EXEC SQL DESCRIBE stmt INTO SQL DESCRIPTOR desc;
41: EXEC SQL GET DESCRIPTOR desc VALUE 1 :name = NAME;

> ecpgtest.pgc:41: WARNING: descriptor ""desc"" does not exist

(Note that the warning mistakenly fires also when the physical order
of ALLOCATE and GET DESCRIPTOR is reversed in a .pgc file.)

I don't come up with an idea how to "fix" it (or I don't find what is
the sane behavior for this feature..), but anyway, I find it hard to
find what to do next from the warning.  It might be helpful that the
warning shows the connection.

> ecpgtest.pgc:41: WARNING: descriptor ""desc"" bound to connection ""c1"" does not exist

(It looks strange that the name is quoted twice but it would be
 another issue.)


ECPGDescribe: SQL_DESCRIBE INPUT_P prepared_name using_descriptor
     {
-        const char *con = connection ? connection : "NULL";
+        const char *con;
+
+        check_declared_list($3);
+        con = connection ? connection : "NULL";
         mmerror(PARSE_ERROR, ET_WARNING, "using unsupported DESCRIBE statement");

Honestly, I don't like the boilerplate. There's no reason for handling
connection at the level.  It seems to me that it is better that the
rule ECPGDescribe passes the parameters force_indicator and stmt name
up to the parent rule-component (stmt:ECPGDescribe) , then the parent
generates the function-call string.


The test portion bloats the patch so it would be easier to read if
that part is separated from the code part.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center