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

From kuroda.hayato@fujitsu.com
Subject RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date
Msg-id TYAPR01MB58664D7C890F631C66D2DAA7F5009@TYAPR01MB5866.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Responses Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Dear Horiguchi-san,

Thank you for replying!

> (Maybe by consulting the code.. Anyway, )

I noticed the patch cannot be applied...

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

I'm not sure about `CREATE TABLE AS EXECUTE`(I'll check your new thread), but at least,
I think `DECLARE CURSOR` uses linked connection.

The following .pgc code:

```pgc
  EXEC SQL CONNECT TO postgres AS connection1;
  EXEC SQL CONNECT TO template1 AS connection2;
  EXEC SQL SET CONNECTION TO connection2;

  EXEC SQL AT connection1 DECLARE sql STATEMENT;
  EXEC SQL PREPARE sql FROM "SELECT current_database()";

  EXEC SQL DECLARE cur CURSOR FOR sql;
  EXEC SQL OPEN cur;
```

will become like this(picked only last two lines):

```c
  /* declare cur cursor for $1 */

  { ECPGdo(__LINE__, 0, 1, "connection1", 0, ECPGst_normal, "declare cur cursor for $1",
        ECPGt_char_variable,(ECPGprepared_statement("connection1", "sql", __LINE__)),(long)1,(long)1,(1)*sizeof(char),
        ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT, ECPGt_EORT);}
```

Of cause, according to [1], the connection is overwritten by check_declared_list()
and it's saved to this->connection.
Please tell me if I misunderstand something.

> 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.

Right. lookup_descriptor() will throw mmerror().

> 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.

I think this phenomenon is quite normal, not bug. When users use connection-associated
prepared_name, it implies using AT clause.
However, I perfectly agree that it's very difficult for users to find a problem from the message.
I will try to add information to it in the next patch.

> 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.

You're right. This is very stupid program. I'll combine them into one.

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

Right, I'll separate and attach again few days. Sorry for inconvenience;-(.

[1]: https://github.com/postgres/postgres/blob/master/src/interfaces/ecpg/preproc/ecpg.trailer#L345

Best Regards,
Hayato Kuroda
FUJITSU LIMITED




pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: postgres_fdw - should we tighten up batch_size, fetch_size options against non-numeric values?
Next
From: Masahiko Sawada
Date:
Subject: Re: Skipping logical replication transactions on subscriber side