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

From: "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date: 2021-07-01 12:55:04
Message-ID: TYAPR01MB58664D7C890F631C66D2DAA7F5009@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2021-07-01 13:00:36 Re: Skipping logical replication transactions on subscriber side
Previous Message Bharath Rupireddy 2021-07-01 12:41:10 Re: postgres_fdw - should we tighten up batch_size, fetch_size options against non-numeric values?