Horton

Thread

Commits

  1. Fix incorrect merge in ECPG code with DECLARE

  2. Fix connection handling for DEALLOCATE and DESCRIBE statements

  3. Add DECLARE STATEMENT command to ECPG

  1. ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-06-25T12:02:22Z

    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? 

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-07-01T08:48:49Z

    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

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-07-01T09:07:06Z

    At Thu, 01 Jul 2021 17:48:49 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> At Fri, 25 Jun 2021 12:02:22 +0000, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> wrote in 
> The following commands handle the liked connection.
> DECLARE
> PREPARE
> EXECUTE
> 
> The follwoing commands don't.
> DESCRIBE
> DEALLOCATE
> DECLARE CURSOR .. FOR
> CREATE TABLE AS EXECUTE

Mmm. It's wrong. CREATE TABLE AS EXECUTE follows.  So DECLARE CURSOR
should follow?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-07-01T12:55:04Z

    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

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-01T16:57:38Z

    "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> writes:
>> 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;-(.

Please also ensure that you're generating the patch against git HEAD.
The cfbot shows it as failing to apply, likely because you're looking
at something predating ad8305a43d1890768a613d3fb586b44f17360f29.

			regards, tom lane

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-07-02T12:53:02Z

    Dear Hackers,

I revised my patch.

> Please also ensure that you're generating the patch against git HEAD.
> The cfbot shows it as failing to apply, likely because you're looking
> at something predating ad8305a43d1890768a613d3fb586b44f17360f29.

Maybe there was something wrong with my local environment. Sorry.

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

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.

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

Check_declared_list() was moved to stmt:ECPGDescribe rule.
Some similar rules still remain in ecpg.trailer, but INPUT/OUTPUT statements have
different rules and actions and I cannot combine well.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Michael Paquier <michael@paquier.xyz> — 2021-07-06T07:58:03Z

    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

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

    Michael Paquier <michael@paquier.xyz> — 2021-07-06T08:04:15Z

    On Tue, Jul 06, 2021 at 04:58:03PM +0900, Michael Paquier wrote:
> 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.

By the way, as DECLARE is new as of v14, I think that the interactions
between DECLARE and the past queries qualify as an open item.  I am
adding Michael Meskes in CC.  I got to wonder how much of a
compatibility break it would be for DEALLOCATE and DESCRIBE to handle
EXEC SQL AT in a way more consistent than DECLARE, even if these are
bounded to a result set, and not a connection.
--
Michael

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-07-06T08:29:27Z

    At Fri, 2 Jul 2021 12:53:02 +0000, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> wrote in 
> Dear Hackers,
> 
> I revised my patch.

Thanks for the new version.

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

Maybe (pre)compile-time check is needed for the descriptor names.
Otherwise we don't notice some of the names are spelled wrongly until
runtime. If it were a string we can live without the check but it is
seemingly an identifier so it is strange that it is not detected at
compile-time. I guess that it is the motivation for the check.

What makes the story complex is that connection matters in the
relation between DESCRIBE and GET DESCRIPTOR. (However, connection
doesn't matter in ALLOCATE DESCRIPTOR..)  Maybe the check involves
connection for this reason.

Since we don't allow descriptors with the same name even if they are
for the different connections, I think we can set the current
connection if any (which is set either by AT option or statement-bound
one) to i->connection silently if i->connection is NULL in
lookup_descriptor(). What do you think about this?

=====
I noticed the following behavior.

> EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> EXEC SQL DESCRIBE stmt INTO SQL DESCRIPTOR mydesc;
> EXEC SQL SET CONNECTION conn2;
ERROR: AT option not allowed in SET CONNECTION STATEMENT

connection is "conn1" at the error time. The parser relies on
output_statement and friends for connection name reset. So the rules
that don't call the functions need to reset it by themselves.

Similary, the following sequence doesn't yield an error, which is
expected.

> EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> EXEC SQL AT conn2 EXECUTE stmt INTO ..;

In this case "conn2" set by the AT option is silently overwritten with
"conn1" by check_declared_list(). I think we should reject AT option
(with a different connection) in that case.



> > You're right. This is very stupid program. I'll combine them into one.
> 
> Check_declared_list() was moved to stmt:ECPGDescribe rule.
> Some similar rules still remain in ecpg.trailer, but INPUT/OUTPUT statements have
> different rules and actions and I cannot combine well.

I'm not sure what you exactly mean by the "INPUT/OUTPUT statements"
but the change of DESCRIBE INPUT/OUTPUT looks fine to me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-07-08T11:42:14Z

    Dear Michael,

> I have been chewing on this comment and it took me some time to
> understand what you meant here.

Sorry... But your understanding is correct.

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

Thank you for giving your argument. I will keep in the next patch.

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

Yeah. I think, at least, DEALLOCATE statement should use the associated connection.


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

Thanks. OK, I'll add them to it.

> By the way, as DECLARE is new as of v14, I think that the interactions
> between DECLARE and the past queries qualify as an open item.  I am
> adding Michael Meskes in CC.  I got to wonder how much of a
> compatibility break it would be for DEALLOCATE and DESCRIBE to handle
> EXEC SQL AT in a way more consistent than DECLARE, even if these are
> bounded to a result set, and not a connection.

I already said above, I think that DEALLOCATE statement should
follow the linked connection, but I cannot decide about DESCRIBE.
I want to ask how do you think.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Michael Paquier <michael@paquier.xyz> — 2021-07-09T02:59:43Z

    On Thu, Jul 08, 2021 at 11:42:14AM +0000, kuroda.hayato@fujitsu.com wrote:
> I already said above, I think that DEALLOCATE statement should
> follow the linked connection, but I cannot decide about DESCRIBE.
> I want to ask how do you think.

I am not completely sure.  It would be good to hear from Michael
Meskes about that, and the introduction of DECLARE makes the barrier
about the use of preferred connections blurrier for those other
queries.
--
Michael

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-07-12T04:05:21Z

    Dear Horiguchi-san,

Thank you for reviewing! I attached new version.
Sorry for delaying reply.

> Since we don't allow descriptors with the same name even if they are
> for the different connections, I think we can set the current
> connection if any (which is set either by AT option or statement-bound
> one) to i->connection silently if i->connection is NULL in
> lookup_descriptor(). What do you think about this?

I tried to implement. Is it correct?

> connection is "conn1" at the error time. The parser relies on
> output_statement and friends for connection name reset. So the rules
> that don't call the functions need to reset it by themselves.

Oh, I didn't notice that. Fixed.
I'm wondering why a output function is not implemented, like output_describe_statement(),
but anyway I put a connection reset in ecpg.addons.

> Similary, the following sequence doesn't yield an error, which is
> expected.
> 
> > EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> > EXEC SQL AT conn2 EXECUTE stmt INTO ..;
> 
> In this case "conn2" set by the AT option is silently overwritten with
> "conn1" by check_declared_list(). I think we should reject AT option
> (with a different connection) in that case.

Actually this comes from Oracle's specification. Pro*C precompiler
overwrite their connection in the situation, hence I followed that.
But I agree this might be confused and I added the warning report.
How do you think? Is it still strange?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    wangsh.fnst@fujitsu.com <wangsh.fnst@fujitsu.com> — 2021-07-20T08:02:44Z

    Hi kuroda-san:

I find another problem about declare statement. The test source looks like:
> EXEC SQL AT con1 DECLARE stmt STATEMENT;
> EXEC SQL PREPARE stmt AS SELECT version();
> EXEC SQL DECLARE cur CURSOR FOR stmt;
> EXEC SQL WHENEVER SQLERROR STOP;

The outout about ecpg:
>test.pgc:14: ERROR: AT option not allowed in WHENEVER statement

After a simple research, I found that after calling function check_declared_list,
the variable connection will be updated, but in some case(e.g. ECPGCursorStmt)
reset connection is missing.

I'm not sure, but how about modify the ecpg.trailer:
> tatement: ecpgstart at toplevel_stmt ';' { connection = NULL; }
> | ecpgstart toplevel_stmt ';'

I think there are lots of 'connection = NULL;' in source, and we should find a 
good location to reset the connection.


Best regards.
Shenhao Wang


-----Original Message-----
From: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com> 
Sent: Monday, July 12, 2021 12:05 PM
To: 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

Dear Horiguchi-san,

Thank you for reviewing! I attached new version.
Sorry for delaying reply.

> Since we don't allow descriptors with the same name even if they are
> for the different connections, I think we can set the current
> connection if any (which is set either by AT option or statement-bound
> one) to i->connection silently if i->connection is NULL in
> lookup_descriptor(). What do you think about this?

I tried to implement. Is it correct?

> connection is "conn1" at the error time. The parser relies on
> output_statement and friends for connection name reset. So the rules
> that don't call the functions need to reset it by themselves.

Oh, I didn't notice that. Fixed.
I'm wondering why a output function is not implemented, like output_describe_statement(),
but anyway I put a connection reset in ecpg.addons.

> Similary, the following sequence doesn't yield an error, which is
> expected.
> 
> > EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> > EXEC SQL AT conn2 EXECUTE stmt INTO ..;
> 
> In this case "conn2" set by the AT option is silently overwritten with
> "conn1" by check_declared_list(). I think we should reject AT option
> (with a different connection) in that case.

Actually this comes from Oracle's specification. Pro*C precompiler
overwrite their connection in the situation, hence I followed that.
But I agree this might be confused and I added the warning report.
How do you think? Is it still strange?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-07-21T08:22:47Z

    Hello, Kuroda-san.

At Mon, 12 Jul 2021 04:05:21 +0000, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> wrote in 
> > Similary, the following sequence doesn't yield an error, which is
> > expected.
> > 
> > > EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> > > EXEC SQL AT conn2 EXECUTE stmt INTO ..;
> > 
> > In this case "conn2" set by the AT option is silently overwritten with
> > "conn1" by check_declared_list(). I think we should reject AT option
> > (with a different connection) in that case.
> 
> Actually this comes from Oracle's specification. Pro*C precompiler
> overwrite their connection in the situation, hence I followed that.
> But I agree this might be confused and I added the warning report.
> How do you think? Is it still strange?

(I'm perplexed from what is done while precompilation and what is done
 at execution time...)

How Pro*C behaves in that case?  If the second command ends with an
error, I think we are free to error out the second command before
execution.  If it works... do you know what is happening at the time?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    Michael Meskes <meskes@postgresql.org> — 2021-07-29T09:22:37Z

    All,

> between DECLARE and the past queries qualify as an open item.  I am
> adding Michael Meskes in CC.  I got to wonder how much of a
> compatibility break it would be for DEALLOCATE and DESCRIBE to handle
> EXEC SQL AT in a way more consistent than DECLARE, even if these are
> bounded to a result set, and not a connection.

I just wanted to let you know that I'm well aware of this thread but
need to get through my backlog before I can comment. Sorry for the
delay.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Peter Geoghegan <pg@bowt.ie> — 2021-07-30T22:01:44Z

    On Thu, Jul 29, 2021 at 12:22 PM Michael Meskes <meskes@postgresql.org> wrote:
> I just wanted to let you know that I'm well aware of this thread but
> need to get through my backlog before I can comment. Sorry for the
> delay.

The RMT discussed this recently. We are concerned about this issue,
including how it has been handled so far.

If you're unable to resolve the issue (or at least commit time to
resolving the issue) then the RMT will call for the revert of the
original feature patch.
-- 
Peter Geoghegan

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-07T01:30:44Z

    On Fri, Jul 30, 2021 at 3:01 PM Peter Geoghegan <pg@bowt.ie> wrote:
> The RMT discussed this recently. We are concerned about this issue,
> including how it has been handled so far.
>
> If you're unable to resolve the issue (or at least commit time to
> resolving the issue) then the RMT will call for the revert of the
> original feature patch.

The RMT continues to be concerned about the lack of progress on this
open item, especially the lack of communication from Michael Meskes,
the committer of the original patch (commit ad8305a). We are prepared
to exercise our authority to resolve open items directly. The only
fallback option available to us is a straight revert of commit
ad8305a.

We ask that Michael Meskes give a status update here no later than
23:59 on Fri 13 Aug ("anywhere on earth" timezone). This update should
include a general assessment of the problem, a proposed resolution
(e.g., committing the proposed patch from Hayato Kuroda), and an
estimate of when we can expect the problem to be fully resolved. If
Michael is unable to provide a status update by that deadline, or if
Michael is unable to commit to a reasonably prompt resolution for this
open item, then the RMT will call for a revert without further delay.

The RMT's first responsibility is to ensure that PostgreSQL 14 is
released on schedule. We would prefer to avoid a revert, but we cannot
allow this to drag on indefinitely.

--
Peter Geoghegan

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-07T08:13:30Z

    Hi Peter,

> The RMT continues to be concerned about the lack of progress on this
> open item, especially the lack of communication from Michael Meskes,
> the committer of the original patch (commit ad8305a). We are prepared
> to exercise our authority to resolve open items directly. The only
> fallback option available to us is a straight revert of commit
> ad8305a.
> 
> We ask that Michael Meskes give a status update here no later than
> 23:59 on Fri 13 Aug ("anywhere on earth" timezone). This update
> should
> include a general assessment of the problem, a proposed resolution
> (e.g., committing the proposed patch from Hayato Kuroda), and an
> estimate of when we can expect the problem to be fully resolved. If
> Michael is unable to provide a status update by that deadline, or if
> Michael is unable to commit to a reasonably prompt resolution for
> this
> open item, then the RMT will call for a revert without further delay.
> 
> The RMT's first responsibility is to ensure that PostgreSQL 14 is
> released on schedule. We would prefer to avoid a revert, but we
> cannot
> allow this to drag on indefinitely.

I get it that the goal is to release PostgreSQL 14 and I also get it
that nobody is interested in the reasons for my slow reaction. I even,
at least to an extend, understand why nobody tried reaching out to me
directly. However, what I cannot understand at all is the tone of this
email. Is this the new way of communication in the PostgreSQL project?

Just to be more precise, I find it highly offensive that you address an
email only to me (everyone else was on CC) and yet do not even try to 
talk to me, but instead talk about me as a third person. I find this
very disrespectful. 

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-07T18:00:24Z

    On Sat, Aug 7, 2021 at 1:13 AM Michael Meskes <meskes@postgresql.org> wrote:
> I get it that the goal is to release PostgreSQL 14 and I also get it
> that nobody is interested in the reasons for my slow reaction. I even,
> at least to an extend, understand why nobody tried reaching out to me
> directly.

That's simply not true. Andrew Dunstan reached out personally and got
no response. He then reached out through a backchannel (a direct
coworker of yours), before finally getting a single terse response
from you here.

Every one of us has a life outside of PostgreSQL. An individual
contributor may not be available, even for weeks at a time. It
happens. The RMT might well have been much more flexible if you
engaged with us privately. There has not been a single iota of
information for us to go on. That's why this happened.

> However, what I cannot understand at all is the tone of this
> email. Is this the new way of communication in the PostgreSQL project?

The tone was formal and impersonal because it represented the position
of the RMT as a whole (not me personally), and because it's a
particularly serious matter for the RMT. It concerned the RMT
exercising its authority to resolve open items directly, in this case
by calling for a revert. This is the option of last resort for us, and
it was important to clearly signal that we had reached that point.

No other committer (certainly nobody on the RMT) knows anything about
ecpg. How much longer were you expecting us to wait for a simple
status update?

-- 
Peter Geoghegan

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-07T19:43:15Z

    > That's simply not true. Andrew Dunstan reached out personally and got
> no response. He then reached out through a backchannel (a direct
> coworker of yours), before finally getting a single terse response
> from you here.

You do know that I did not receive any email from Andrew. After all I
explained this to the backchannel you mentioned. I do not know what
happened, I do not even know if it was one email or several, but I
checked everything, there simply is no such email in my mailbox. 

> Every one of us has a life outside of PostgreSQL. An individual
> contributor may not be available, even for weeks at a time. It
> happens. The RMT might well have been much more flexible if you
> engaged with us privately. There has not been a single iota of
> information for us to go on. That's why this happened.

Again, I didn't know the RMT was expecting anything from me. Yes, I
knew I needed to spend some time on a technical issues, but that's
exactly the information I had at the time.

> 
> The tone was formal and impersonal because it represented the
> position
> of the RMT as a whole (not me personally), and because it's a
> particularly serious matter for the RMT. It concerned the RMT
> exercising its authority to resolve open items directly, in this case
> by calling for a revert. This is the option of last resort for us,
> and
> it was important to clearly signal that we had reached that point.

Please read my prior email completely, I did go into detail about what
I meant with tone. I don't mind a formal wording and I completely agree
that a decision has to be made at some point. I was wrong in thinking
there was more time left, but that's also not the point. The point is
that you talk *about* me in the third person in an email you address at
me. It might be normal for you, but in my neck of the woods this is
very rude behavior. 

> No other committer (certainly nobody on the RMT) knows anything about
> ecpg. How much longer were you expecting us to wait for a simple
> status update?

Where did I say I expect you to wait? How could I even do that given
that I didn't even know you were waiting for a status update from me?

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Andrew Dunstan <andrew@dunslane.net> — 2021-08-07T20:49:18Z

    On 8/7/21 3:43 PM, Michael Meskes wrote:
>
>> No other committer (certainly nobody on the RMT) knows anything about
>> ecpg. How much longer were you expecting us to wait for a simple
>> status update?
> Where did I say I expect you to wait? How could I even do that given
> that I didn't even know you were waiting for a status update from me?
>

Michael,


During the Beta period, every open item is the responsibility of the
relevant committer. There is an expectation that each item will be dealt
with in a timely fashion. It is the RMT's responsibility to monitor the
open items list and take action if any item on the list endangers the
timing or stability of the release.

In the present instance all we have had from you is a terse statement
that you were under pressure of work, with the implication that you
would deal with the item in an unspecified way at an unspecified time in
the future. We don't think that meets the requirements I stated above.

W.r.t. previous contact regarding this item, I sent you email on July
23rd. It was addressed to  <mailto:meskes@postgresql.org> and had this text:

> Michael,
>
>
> This is an open item for release 14. Please let us know if you are going
> to attend to it.
>

Other committers with items on the list can probably testify to the fact
that we have dropped them similar notes via email or messenger app, so
you're certainly not being singled out.

Peter followed up to your eventual note on the list on the 30th. So
we've taken substantial steps to make you aware of what is expected.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-07T22:31:49Z

    On Sat, Aug 7, 2021 at 12:43 PM Michael Meskes <meskes@postgresql.org> wrote:
> Again, I didn't know the RMT was expecting anything from me. Yes, I
> knew I needed to spend some time on a technical issues, but that's
> exactly the information I had at the time.

As Andrew mentioned, I sent you an email on the 30th -- a full week
prior to the email that formally timeboxed this open item. That
earlier email is here:

https://postgr.es/m/CAH2-Wzk=QxtSp0H5EKV92EH0u22HVMQLHGwYP4_FA3yTiEi9Yg@mail.gmail.com

I really don't know why you're surprised that the issue came to a head
with yesterday's email. This earlier email was similar in tone, and
yet went completely unanswered for a full week. This situation has
been steadily escalating for quite a while now.

> Please read my prior email completely, I did go into detail about what
> I meant with tone. I don't mind a formal wording and I completely agree
> that a decision has to be made at some point. I was wrong in thinking
> there was more time left, but that's also not the point. The point is
> that you talk *about* me in the third person in an email you address at
> me. It might be normal for you, but in my neck of the woods this is
> very rude behavior.

I also talked about the RMT in the third person. My intent was to make
the message legalistic and impersonal. That's what is driving our
thinking on this -- the charter of the RMT.

The RMT primarily exists to resolve open items that risk holding up
the release. When any committer of any patch simply doesn't respond in
any substantive way to the RMT (any RMT), the RMT is all but forced to
fall back on the crude option of reverting the patch. I cannot imagine
any other outcome if other individuals were involved, or if the
details were varied.

We're all volunteers, just like you. I happen to be a big believer in
our culture of personal ownership and personal responsibility. But you
simply haven't engaged with us at all.

> Where did I say I expect you to wait? How could I even do that given
> that I didn't even know you were waiting for a status update from me?

You didn't say anything at all, which speaks for itself. And makes it
impossible for us to be flexible.

--
Peter Geoghegan

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-08T18:34:53Z

    On Sat, 2021-08-07 at 15:31 -0700, Peter Geoghegan wrote:
> On Sat, Aug 7, 2021 at 12:43 PM Michael Meskes
> <meskes@postgresql.org> wrote:
> > Again, I didn't know the RMT was expecting anything from me. Yes, I
> > knew I needed to spend some time on a technical issues, but that's
> > exactly the information I had at the time.
> 
> As Andrew mentioned, I sent you an email on the 30th -- a full week
> prior to the email that formally timeboxed this open item. That
> earlier email is here:
> 
> https://postgr.es/m/CAH2-Wzk=QxtSp0H5EKV92EH0u22HVMQLHGwYP4_FA3yTiEi9Yg@mail.gmail.com

This email said nothing about sending a status update or a deadline or
any question at all, only that you'd revert the patch if I was unable
to resolve the issue. So what's your point? 


> I also talked about the RMT in the third person. My intent was to
> make

So? It's okay to disrespect a person if you mention the team that you
are representing in the third person, too?

> the message legalistic and impersonal. That's what is driving our
> thinking on this -- the charter of the RMT.

Please show me where the charter says that disrespecting a person is
fine. And while we're add it, does the code of conduct say anything
about the way people should be treated? 

> We're all volunteers, just like you. I happen to be a big believer in
> our culture of personal ownership and personal responsibility. But
> you
> simply haven't engaged with us at all.

Which I tried to explain several times, but apparently not well enough.
Let me give you a short rundown from my perspective:

- A patch is sent that I mistakenly thought was a new feature and thus
did not apply time too immediately.
- After a while I get an email from you as spokesperson of the RMT that
if this is not fixed it'll have to be reverted eventually.
- I learn that Andrew tried to reach me. Again, I believe you Andrew,
that you sent the email, but I never saw it. Whether it's some
filtering or a user error that made it disappear, I have no idea, but
I'm surely sorry about that.
- I receive that email we keep talking about, the one in which you
treat me like I'm not even worth being addressed.

> You didn't say anything at all, which speaks for itself. And makes it
> impossible for us to be flexible.

Which flexibility did I ask for? It'd be nice if you only accused me of
things I did.

Honestly I do not understand you at all. You keep treating me like I
was asking for anything unreasonable while I'm only trying to explain
why I didn't act earlier. The only issue I have is the rude treatment
you gave me. 

Just for the record, of course I'm going to look into the issue before
your deadline and will send a status update.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-08T19:48:44Z

    On Sun, Aug 8, 2021 at 11:34 AM Michael Meskes <meskes@postgresql.org> wrote:
> > https://postgr.es/m/CAH2-Wzk=QxtSp0H5EKV92EH0u22HVMQLHGwYP4_FA3yTiEi9Yg@mail.gmail.com
>
> This email said nothing about sending a status update or a deadline or
> any question at all, only that you'd revert the patch if I was unable
> to resolve the issue. So what's your point?

I think that it's crystal clear what I meant in the email of July 30.
I meant: it's not okay that you're simply ignoring the RMT. You hadn't
even made a token effort at that point. For example you didn't give
the proposed fix a cursory 15 minute review, just so we had some very
rough idea of where things stand. You still haven't.

My understanding of what you're taking issue with (perhaps a flawed
understanding) is that you think that you have been treated unfairly
or arbitrarily in general. That's why I brought up the email of July
30 yesterday. So my point was: no, you haven't been treated unfairly.
If you only take issue with the specific tone and tenor of my email
from Friday (the email that specified a deadline), and not the content
itself, then maybe the timeline and the wider context are not so
important.

I am still unsure about whether your concern is limited to the tone of
the email from Friday, or if you also take exception to the content of
that email (and the wider context).

> > I also talked about the RMT in the third person. My intent was to
> > make
>
> So? It's okay to disrespect a person if you mention the team that you
> are representing in the third person, too?

Perhaps the tone of my email from Friday was unhelpful. Even still, I
am surprised that you seem to think that it was totally outrageous --
especially given the context. It was the first email that you
responded to *at all* on this thread, with the exception of your
original terse response. I am not well practised in communicating with
a committer that just doesn't engage with the RMT at all. This is all
new to me. I admit that I found it awkward to write the email for my
own reasons.

> > You didn't say anything at all, which speaks for itself. And makes it
> > impossible for us to be flexible.
>
> Which flexibility did I ask for? It'd be nice if you only accused me of
> things I did.

I brought up flexibility to point out that this could have been
avoided. If you needed more time, why didn't you simply ask for it?

Again, the scope of what you're complaining about was (and still is)
unclear to me.

> Just for the record, of course I'm going to look into the issue before
> your deadline and will send a status update.

Thank you.

-- 
Peter Geoghegan

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T07:10:44Z

    > I think that it's crystal clear what I meant in the email of July 30.
> I meant: it's not okay that you're simply ignoring the RMT. You
> hadn't
> even made a token effort at that point. For example you didn't give
> the proposed fix a cursory 15 minute review, just so we had some very
> rough idea of where things stand. You still haven't.

How do you know I didn't spend 15 minutes looking at the patch and the
whole email thread? I surely did and it was more than 15 minutes, but
not enough to give a meaningful comment. If you can do it in 15
minutes, great for you, I cannot.

The meaning of your email of July 30 was crystal clear, yes. It means
you'd revert the patch if I didn't resolve the issue. This is literally
what it says. If you meant to say "It's not okay that you're simply
ignoring the RMT. You hadn't even made a token effort at that point."
it might have been helpful if you said that, instead of having me guess
if there was a hidden meaning in your email.

Besides, I have not ignored the RMT. I don't know why you keep
insisting on something that is simply not true.

> My understanding of what you're taking issue with (perhaps a flawed
> understanding) is that you think that you have been treated unfairly
> or arbitrarily in general. That's why I brought up the email of July
> 30 yesterday. So my point was: no, you haven't been treated unfairly.

Yes, this is a flawed understanding. I'm sorry you came to that
understanding, I though my emails were pretty clear as to what I was
objecting to.

> If you only take issue with the specific tone and tenor of my email
> from Friday (the email that specified a deadline), and not the
> content
> itself, then maybe the timeline and the wider context are not so
> important.
> 
> I am still unsure about whether your concern is limited to the tone
> of
> the email from Friday, or if you also take exception to the content
> of
> that email (and the wider context).

At the risk of repeating myself, my concern is *only* the rude and
disrespectful way of addressing me in the third person while talking to
me directly. Again, I though I made that clear in my first email about
the topic and every one that followed.

> Perhaps the tone of my email from Friday was unhelpful. Even still, I
> am surprised that you seem to think that it was totally outrageous --
> especially given the context. It was the first email that you

The context never makes a derogative communication okay, at least not
in my opinion.

> responded to *at all* on this thread, with the exception of your
> original terse response. I am not well practised in communicating
> with
> a committer that just doesn't engage with the RMT at all. This is all
> new to me. I admit that I found it awkward to write the email for my
> own reasons.

I was *never* asked for *any* response in *any* email, save the
original technical discussion, which I did answer with telling people
that I'm lacking time but will eventually get to it. Just to be
precise, your email from July 30 told me and everyone how this will be
handled. A reasonable procedure, albeit not one we'd like to see
happen. But why should I answer and what? It's not that you bring this
up as a discussion point, but as a fact.

> I brought up flexibility to point out that this could have been
> avoided. If you needed more time, why didn't you simply ask for it?

The first conversation that brought up the time issue was your email
that started this thread. There you set a deadline which I understand
and accept. But then I never said a word against it, so the question
remains, why accusing me of something I never did?

> Again, the scope of what you're complaining about was (and still is)
> unclear to me.

I'm sorry, but I have no idea how to explain it more clearly. I'm not
asking for any favor or special treatment, I just ask to be treated
like a person.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-08-09T17:00:56Z

    Dear Hackers,

I perfectly missed mails and 8/9 was a national holiday.
I must apologize about anything. Very sorry for inconvenience.

> The RMT's first responsibility is to ensure that PostgreSQL 14 is
> released on schedule. We would prefer to avoid a revert, but we cannot
> allow this to drag on indefinitely.

Of cause I will try to avoid it but I can understand doing your business.

Dear Meskes,

I summarize the thread.
As you might know DECLARE STATEMENT has been added from PG14, but I
found that connection-control feature cannot be used for DEALLOCATE
and DESCRIBE statement (More details, please see patches or ask me).
But we have a question - what statement should use the associated
connection? Obviously DEALLOCATE statement should follow the linked
connection because the statement uses only one sql identifier. In
DESCRIBE or any other descriptor-related statements, however, I think
it is non-obvious because they have also descriptor-name. Currently I
made patches that includes about DESCRIBE, but I'm wondering this
approach is correct. I want you to ask your opinion about the scope of
DECLARE STATEMENT.
Coding is not hard hence I think we can fix this until the end of Sep.
if we set a policy correctly and have reviewers.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-08-09T17:01:36Z

    Dear Wang,

Good reporting!

> I'm not sure, but how about modify the ecpg.trailer:
> > statement: ecpgstart at toplevel_stmt ';' { connection = NULL; }
> > | ecpgstart toplevel_stmt ';'
> I think there are lots of 'connection = NULL;' in source, and we should find a
good location to reset the connection.

Thank you for giving a solution! I will consider the idea and
integrate it if it's OK.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-08-09T17:01:57Z

    Dear Horiguchi-san,

> How Pro*C behaves in that case? If the second command ends with an
> error, I think we are free to error out the second command before
> execution. If it works... do you know what is happening at the time?

You asked that how Oracle works when the followings precompiled and
executed, don't it?
> > > EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> > > EXEC SQL AT conn2 EXECUTE stmt INTO ..;

 While precompiling, it does not throw any errors. While executing,
the second statement will execute at conn1 without warnings.
So the added warning is postgres-extended.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T18:21:00Z

    Dear Kuroda-san,

> I perfectly missed mails and 8/9 was a national holiday.
> I must apologize about anything. Very sorry for inconvenience.

No need to, non of it is your fault.

> I summarize the thread.

Thank you so much, this is very, very helpful.

> As you might know DECLARE STATEMENT has been added from PG14, but I
> found that connection-control feature cannot be used for DEALLOCATE
> and DESCRIBE statement (More details, please see patches or ask me).
> But we have a question - what statement should use the associated
> connection? Obviously DEALLOCATE statement should follow the linked
> connection because the statement uses only one sql identifier. In
> DESCRIBE or any other descriptor-related statements, however, I think
> it is non-obvious because they have also descriptor-name. Currently I
> made patches that includes about DESCRIBE, but I'm wondering this
> approach is correct. I want you to ask your opinion about the scope
> of
> DECLARE STATEMENT.

I've been reading through quite a few documents to come up with a good
reason one way or the other, but as you already pointed out yourself,
other database systems seem to not be consequent about the usage
either. 

Unfortunately I didn't find my copy of the SQL standard. But then I
kind of doubt it has much to say about this particular issue.

Anyway, I'm currently leaning towards including DESCRIBE in the list of
statements that are influenced by DECLARE STATEMENT. My reason is that
DESCRIBE can be issued with an AT clause already, regardless whether it
makes sense or not. Or in other words, if we allow it to get a
connection name one way, why should we forbid the other way. To me this
seems to be more confusing than the current situation.

The alternative would be to forbid using an AT clause with DESCRIBE,
which would definitely be a compatibility change, although, again, not
a functional one.

> Coding is not hard hence I think we can fix this until the end of
> Sep.
> if we set a policy correctly and have reviewers.

Fully agreed. That's why a short glance at the patch didn't suffice to
answer this. I didn't see any issues with the patch so far. Please send
it to me once its finished (or is it already?) and I'll give it a run,
too.

Hopefully I caught up on all emails and didn't miss parts of the
thread.

Thanks,
Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-09T18:30:02Z

    On Mon, Aug 9, 2021 at 12:10 AM Michael Meskes <meskes@postgresql.org> wrote:
> How do you know I didn't spend 15 minutes looking at the patch and the
> whole email thread? I surely did and it was more than 15 minutes, but
> not enough to give a meaningful comment. If you can do it in 15
> minutes, great for you, I cannot.

That was just an example of a token response. I don't know anything about ecpg.

> Besides, I have not ignored the RMT. I don't know why you keep
> insisting on something that is simply not true.

My email of July 30 was itself pretty strongly worded, but went
unanswered for a full week. Not even a token response. If that doesn't
count as "ignoring the RMT", then what does? How much time has to pass
before radio silence begins to count as "ignoring the RMT", in your
view of things? A month? More?

> At the risk of repeating myself, my concern is *only* the rude and
> disrespectful way of addressing me in the third person while talking to
> me directly. Again, I though I made that clear in my first email about
> the topic and every one that followed.

Okay, I understand that now.

> I was *never* asked for *any* response in *any* email, save the
> original technical discussion, which I did answer with telling people
> that I'm lacking time but will eventually get to it. Just to be
> precise, your email from July 30 told me and everyone how this will be
> handled. A reasonable procedure, albeit not one we'd like to see
> happen. But why should I answer and what? It's not that you bring this
> up as a discussion point, but as a fact.

As Andrew pointed out, there is a general expectation that committers
take care of their own open items. That doesn't mean that they are
obligated to personally fix bugs. Just that the final responsibility
to make sure that the issue is resolved rests with the committer. This
is one of the few hard rules that we have.

As I've said before, RMT-driven revert is something that I see as an
option of last resort. The RMT charter doesn't go quite that far, but
I'd argue that my interpretation is quite natural given how committer
responsibility works in general. In other words, I personally believe
that our bottom-up approach is on balance a good one, and should be
preserved.

Maybe the issue is muddied by the fact that we each have different
views of the community process, at a high level (I'm unsure). Unlike
you, I don't believe that RMT-driven revert is "a reasonable
procedure". I myself see the RMT's power to resolve open items in this
way as a necessary evil. Something to be avoided at all costs. Why
should people that don't know anything about ecpg make decisions about
ecpg? In general they should not.

-- 
Peter Geoghegan

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T18:45:17Z

    > My email of July 30 was itself pretty strongly worded, but went
> unanswered for a full week. Not even a token response. If that
> doesn't
> count as "ignoring the RMT", then what does? How much time has to
> pass
> before radio silence begins to count as "ignoring the RMT", in your
> view of things? A month? More?

If you want me to answer, how about asking a question? Or telling me
that you'd like some feedback? I don't see how I should know that you
expect a reply to a simple statement of facts.

> Okay, I understand that now.

And? Do you care at all?

> As Andrew pointed out, there is a general expectation that committers
> take care of their own open items. That doesn't mean that they are
> obligated to personally fix bugs. Just that the final responsibility
> to make sure that the issue is resolved rests with the committer.
> This
> is one of the few hard rules that we have.

Sure, I don't question that. Again, I knew about the issue, only
misjudged it in the beginning. Your email from July 30 did show me that
it was more urgent but still didn't create the impression that there
was such a short deadline. In my opinion my prior email already
explained that I was on it, but couldn't give an estimate.

> As I've said before, RMT-driven revert is something that I see as an
> option of last resort. The RMT charter doesn't go quite that far, but
> I'd argue that my interpretation is quite natural given how committer
> responsibility works in general. In other words, I personally believe
> that our bottom-up approach is on balance a good one, and should be
> preserved.

Fair enough, to me a revert is a revert, no matter who issues it.

> Maybe the issue is muddied by the fact that we each have different
> views of the community process, at a high level (I'm unsure). Unlike
> you, I don't believe that RMT-driven revert is "a reasonable
> procedure". I myself see the RMT's power to resolve open items in
> this
> way as a necessary evil. Something to be avoided at all costs. Why
> should people that don't know anything about ecpg make decisions
> about
> ecpg? In general they should not.

Well, you did lay out what the decision would be and I fully agreed
with it. So again, what was there to do? Had you asked me if I agreed,
I would told you. 

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-09T19:30:39Z

    On Mon, Aug 9, 2021 at 11:45 AM Michael Meskes <meskes@postgresql.org> wrote:
> If you want me to answer, how about asking a question? Or telling me
> that you'd like some feedback? I don't see how I should know that you
> expect a reply to a simple statement of facts.

I expressed concern in fairly strong terms, and received no answer for
a full week. I consider that "ignoring the RMT", but you take issue
with that characterization because my email didn't ask an explicit
question with a question mark. Despite the fact that it is generally
understood that "committers own their own items", and that the RMT
exists as a final check on that.

Clearly we disagree about this. I don't think that there is anything
to be gained from discussing this any further, though. I suggest that
we leave it at that.

> > Okay, I understand that now.
>
> And? Do you care at all?

I don't want to upset anybody for any reason. I regret that my words
have upset you, but I think that they were misinterpreted in a way
that I couldn't possibly have predicted. The particular aspect of last
Friday's email that you took exception to was actually intended to
convey that it was not personal. Remember, my whole ethos is to avoid
strong RMT intervention when possible, to make it impersonal. My
framing of things had the opposite effect to the one I'd intended,
ironically.

> Sure, I don't question that. Again, I knew about the issue, only
> misjudged it in the beginning. Your email from July 30 did show me that
> it was more urgent but still didn't create the impression that there
> was such a short deadline. In my opinion my prior email already
> explained that I was on it, but couldn't give an estimate.

How could anybody on the RMT judge what was going on sensibly? There
was *zero* information from you (the original committer, our point of
contact) about an item that is in a totally unfamiliar part of the
code to every other committer. We were effectively forced to make very
conservative assumptions about the deadline. I think that it's very
likely that this could have been avoided if only you'd engaged to some
degree -- if you had said it was a short deadline then we'd likely
have taken your word for it, as the relevant subject matter expert and
committer in charge of the item. But we were never given that choice.

> Well, you did lay out what the decision would be and I fully agreed
> with it. So again, what was there to do? Had you asked me if I agreed,
> I would told you.

If the patch being reverted was so inconsequential to you that you
didn't even feel the need to write a brief email about it, why did you
commit it in the first place? I just don't understand this at all.

-- 
Peter Geoghegan

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

    Robert Haas <robertmhaas@gmail.com> — 2021-08-09T20:28:25Z

    On Sat, Aug 7, 2021 at 4:13 AM Michael Meskes <meskes@postgresql.org> wrote:
> I get it that the goal is to release PostgreSQL 14 and I also get it
> that nobody is interested in the reasons for my slow reaction. I even,
> at least to an extend, understand why nobody tried reaching out to me
> directly. However, what I cannot understand at all is the tone of this
> email. Is this the new way of communication in the PostgreSQL project?
>
> Just to be more precise, I find it highly offensive that you address an
> email only to me (everyone else was on CC) and yet do not even try to
> talk to me, but instead talk about me as a third person. I find this
> very disrespectful.

Hi,

FWIW, I don't think that the phrasing of Peter's email is
disrespectful. As I read it, it simply states that the RMT has made a
decision to revert the patch unless certain assurances are given
before a certain date. I don't expect anyone will particularly like
receiving such an email, because nobody likes to be threatened with a
revert, but I don't think there is anything rude about it. Either you
are willing to commit to resolving the problem by a date that the RMT
finds acceptable, or you aren't. If you are, great. If you aren't, the
patch is going to get reverted. That sucks, but it's nothing against
you personally; it's just what happens sometimes. I also feel rather
strongly that being a member of the RMT is a pretty thankless task,
involving going through a lot of patches that you may not care about
and trying to make decisions that will benefit the project, even while
knowing that some people may not like them. We should give people who
are willing to offer such service the benefit of the doubt.

On the substance of the issue, one question that I have reading over
this thread is whether there's really a bug here at all. It is being
represented (and I have not checked whether this is accurate) that the
patch affects the behavior of  DECLARE, PREPARE, and EXECUTE, but not
DESCRIBE, DEALLOCATE, DECLARE CURSOR .. FOR, or CREATE TABLE AS
EXECUTE. However, it also seems that it's not entirely clear what the
behavior ought to be in those cases, except perhaps for DESCRIBE. If
that's the case, maybe this doesn't really need to be an open item,
and maybe we don't therefore need to talk about whether it should be
reverted. Maybe it's just a feature that supports certain things now
and in the future, after due reflection, perhaps it will support more.

I would be interested in hearing your view, and that of others, on
whether this is really a bug at all.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T20:38:07Z

    > question with a question mark. Despite the fact that it is generally
> understood that "committers own their own items", and that the RMT
> exists as a final check on that.

This does not contradict my opinion, but anyway. 

> Clearly we disagree about this. I don't think that there is anything
> to be gained from discussing this any further, though. I suggest that
> we leave it at that.

Agreed.

> I don't want to upset anybody for any reason. I regret that my words
> have upset you, but I think that they were misinterpreted in a way
> that I couldn't possibly have predicted. The particular aspect of

I strongly object to that. It's pretty obvious to me that addressing
people in third person is very offending.

> last
> Friday's email that you took exception to was actually intended to
> convey that it was not personal. Remember, my whole ethos is to avoid
> strong RMT intervention when possible, to make it impersonal. My
> framing of things had the opposite effect to the one I'd intended,
> ironically.

Let me stress again that the third person part is the bad thing in my
opinion, not the rest of the words.
 
> How could anybody on the RMT judge what was going on sensibly? There
> was *zero* information from you (the original committer, our point of
> contact) about an item that is in a totally unfamiliar part of the
> code to every other committer. We were effectively forced to make
> very
> conservative assumptions about the deadline. I think that it's very
> likely that this could have been avoided if only you'd engaged to
> some
> degree -- if you had said it was a short deadline then we'd likely
> have taken your word for it, as the relevant subject matter expert
> and
> committer in charge of the item. But we were never given that choice.

The same holds the other way round, I only understood later that you
wanted more information. Had I known that earlier, I would have gladly
given them. 

> > Well, you did lay out what the decision would be and I fully agreed
> > with it. So again, what was there to do? Had you asked me if I
> > agreed,
> > I would told you.
> 
> If the patch being reverted was so inconsequential to you that you
> didn't even feel the need to write a brief email about it, why did
> you
> commit it in the first place? I just don't understand this at all.

I'm getting very tired of you accusing me of things I neither said nor
did. Please stop doing that or show me the email where I said the patch
was "inconsequential"? As for writing a brief email, please read all
the other emails in this thread, I've explained myself repeatedly
already.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T20:50:29Z

    Hi,

> FWIW, I don't think that the phrasing of Peter's email is
> disrespectful. As I read it, it simply states that the RMT has made a

As I said before, it might be a cultural difference. What I don't
understand is, that a simple "Hi Michael, this is what the RMT
decided:" would have been sufficient to make this email okay. I take
offense in being addressed in third person only.

> strongly that being a member of the RMT is a pretty thankless task,

That I agree with.

> On the substance of the issue, one question that I have reading over
> this thread is whether there's really a bug here at all. It is being
> represented (and I have not checked whether this is accurate) that
> the
> patch affects the behavior of  DECLARE, PREPARE, and EXECUTE, but not
> DESCRIBE, DEALLOCATE, DECLARE CURSOR .. FOR, or CREATE TABLE AS
> EXECUTE. However, it also seems that it's not entirely clear what the
> behavior ought to be in those cases, except perhaps for DESCRIBE. If
> that's the case, maybe this doesn't really need to be an open item,
> and maybe we don't therefore need to talk about whether it should be
> reverted. Maybe it's just a feature that supports certain things now
> and in the future, after due reflection, perhaps it will support
> more.

The way I see it we should commit the patch that makes more statements
honor the new DECLARE STATEMENT feature. I don't think we can change
anything for the worse by doing that. And other databases are not
really strict about this either.

> I would be interested in hearing your view, and that of others, on
> whether this is really a bug at all.

I think the question is more which version of the patch we commit as it
does increase the functionality of ecpg.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    David G. Johnston <david.g.johnston@gmail.com> — 2021-08-09T20:55:51Z

    On Mon, Aug 9, 2021 at 1:38 PM Michael Meskes <meskes@postgresql.org> wrote:

> > I don't want to upset anybody for any reason. I regret that my words
> > have upset you, but I think that they were misinterpreted in a way
> > that I couldn't possibly have predicted. The particular aspect of
>
> I strongly object to that. It's pretty obvious to me that addressing
> people in third person is very offending.
>

And using the third person to avoid making things personal, and properly
represent one's role as a representative as opposed to an individual, is
something at least two of us consider to be "professional".  If others
taking on a professional/formal tone with you is offending I politely
suggest you need to at least cut them some slack when you haven't informed
them of this previously.  Cultural differences happen in both directions.

> How could anybody on the RMT judge what was going on sensibly? There
> > was *zero* information from you (the original committer, our point of
> > contact) about an item that is in a totally unfamiliar part of the
> > code to every other committer. We were effectively forced to make
> > very
> > conservative assumptions about the deadline. I think that it's very
> > likely that this could have been avoided if only you'd engaged to
> > some
> > degree -- if you had said it was a short deadline then we'd likely
> > have taken your word for it, as the relevant subject matter expert
> > and
> > committer in charge of the item. But we were never given that choice.
>
> The same holds the other way round, I only understood later that you
> wanted more information. Had I known that earlier, I would have gladly
> given them.


There is clearly an expectation from the RMT, and at least myself, that:

"The RMT discussed this recently. We are concerned about this issue,
including how it has been handled so far.

If you're unable to resolve the issue (or at least commit time to
resolving the issue) then the RMT will call for the revert of the
original feature patch."

is expected to elicit a response from the comitter in a timely fashion.
Really, any email sent to -hackers from the RMT about a specific commit; or
even any email sent to -hackers by a core team member, is expected to be
responded to in a timely manner.  These teams should not be getting
involved with the day-to-day operations and being responsive to them is
part of the obligation of being a committer.

In hindsight probably the quoted email above should have been worded.

"If you're unable to resolve the issue, or communicate a timely plan for
doing so, within the next week please revert the patch."

Making it clear that the committer should be the one performing the
revert.  Then, absent feedback or a revert, the second email and the RMT
team performing the revert, would be appropriate.

David J.

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-09T21:04:50Z

    On Mon, Aug  9, 2021 at 10:38:07PM +0200, Michael Meskes wrote:
> > Clearly we disagree about this. I don't think that there is anything
> > to be gained from discussing this any further, though. I suggest that
> > we leave it at that.
> 
> Agreed.
> 
> > I don't want to upset anybody for any reason. I regret that my words
> > have upset you, but I think that they were misinterpreted in a way
> > that I couldn't possibly have predicted. The particular aspect of
> 
> I strongly object to that. It's pretty obvious to me that addressing
> people in third person is very offending.

So, you object to him referring to you in the third person in an email,
and you object to him saying it was "misinterpreted".  Are you going to
object to my email too?

I think everyone can accept that you interpreted what Peter said as
offensive, but you must also give the same acceptance that someone might
not consider it offensive.  For example, I did not read it as offensive
at all.

I think it might have been in the third person because at that point,
Peter didn't expect a reply from you, and put you on the "TO" line
merely as a courtesy.  He could have put out an email about reverting
the patch without you on the email header at all, I guess --- then he
could have referred to you without offending you.

> > How could anybody on the RMT judge what was going on sensibly? There
> > was *zero* information from you (the original committer, our point of
> > contact) about an item that is in a totally unfamiliar part of the
> > code to every other committer. We were effectively forced to make
> > very
> > conservative assumptions about the deadline. I think that it's very
> > likely that this could have been avoided if only you'd engaged to
> > some
> > degree -- if you had said it was a short deadline then we'd likely
> > have taken your word for it, as the relevant subject matter expert
> > and
> > committer in charge of the item. But we were never given that choice.
> 
> The same holds the other way round, I only understood later that you
> wanted more information. Had I known that earlier, I would have gladly
> given them. 

Let me be practical here --- the more someone has to be chased for a
reply, the less confidence they have in that person.  If the RMT
contacts you about something, and obviously they have had to take usual
efforts to contact you, the more it is on you to give a full report and
a timeline of when you will address the issue.  If they had to chase you
around, and you gave them a short answer, the less confidence they have
in this getting resolved in a timely manner.

It is the RMT's responsibility to resolve things in a timely manner, and
since they have contacted you, you should be going out of your way to at
least give them confidence that it will be dealt with by you, rather
than them.  Whether the problem is them not asking for a timeline or you
not offering one, the real solution would have been to provide a
timeline to them when they contacted you, since if the RMT is contacting
you, it is a serious issue.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-09T21:05:32Z

    On Mon, Aug 9, 2021 at 1:38 PM Michael Meskes <meskes@postgresql.org> wrote:
> > I don't want to upset anybody for any reason. I regret that my words
> > have upset you, but I think that they were misinterpreted in a way
> > that I couldn't possibly have predicted. The particular aspect of
>
> I strongly object to that. It's pretty obvious to me that addressing
> people in third person is very offending.

I think that this must be a cultural thing. I can see how somebody
would see the third person style as overly formal or stilted. An
interpretation like that does make sense to me. But I knew of no
reason why you might find that style made the message offensive. It
was never intended to denigrate.

I don't know you all that well, but we have talked for quite a while
on a few occasions. I got the sense that you are a decent person from
these conversations. I have no possible reason to denigrate or insult
you. In general I try to be respectful, and if I ever fail it's not
because I didn't care at all. Anybody that knows me well knows that I
am not a mean spirited person.

If this is just an unfortunate misunderstanding, as I suspect it is,
then I would be happy to let it go, and treat it as something to learn
from.

-- 
Peter Geoghegan

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T21:48:07Z

    > > > I don't want to upset anybody for any reason. I regret that my
> > > words
> > > have upset you, but I think that they were misinterpreted in a
> > > way
> > > that I couldn't possibly have predicted. The particular aspect of
> > 
> > I strongly object to that. It's pretty obvious to me that
> > addressing
> > people in third person is very offending.
> 
> So, you object to him referring to you in the third person in an
> email,
> and you object to him saying it was "misinterpreted".  Are you going
> to
> object to my email too?

No, of course not. And sorry for not being precise enough, I only
objected to the prediction part, but I agree, I take the objection
back. I guess it's as difficult for Peter to understand why this is
offensive as it is for me to not see it as such.

> I think it might have been in the third person because at that point,
> Peter didn't expect a reply from you, and put you on the "TO" line
> merely as a courtesy.  He could have put out an email about reverting
> the patch without you on the email header at all, I guess --- then he
> could have referred to you without offending you.

Right, that was my only problem originally. It seemed difficult to
bring that point over.

> Let me be practical here --- the more someone has to be chased for a
> reply, the less confidence they have in that person.  If the RMT
> contacts you about something, and obviously they have had to take
> usual
> efforts to contact you, the more it is on you to give a full report
> and
> a timeline of when you will address the issue.  If they had to chase
> you
> around, and you gave them a short answer, the less confidence they
> have
> in this getting resolved in a timely manner.

Again agreed, please keep in mind, though, that I didn't notice I was
being chased until Peter's first email.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-09T22:00:00Z

    On Mon, Aug  9, 2021 at 11:48:07PM +0200, Michael Meskes wrote:
> No, of course not. And sorry for not being precise enough, I only
> objected to the prediction part, but I agree, I take the objection
> back. I guess it's as difficult for Peter to understand why this is
> offensive as it is for me to not see it as such.

OK, good.

> > Let me be practical here --- the more someone has to be chased for a
> > reply, the less confidence they have in that person.  If the RMT
> > contacts you about something, and obviously they have had to take
> > usual
> > efforts to contact you, the more it is on you to give a full report
> > and
> > a timeline of when you will address the issue.  If they had to chase
> > you
> > around, and you gave them a short answer, the less confidence they
> > have
> > in this getting resolved in a timely manner.
> 
> Again agreed, please keep in mind, though, that I didn't notice I was
> being chased until Peter's first email.

I was asked by the RMT to contact one of your employees, and I did, to
tell you that the RMT was looking for feedback from you on an ecpg
issue.  Not sure if that was before or after Peter's email.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T22:03:24Z

    Peter,

> I think that this must be a cultural thing. I can see how somebody
> would see the third person style as overly formal or stilted. An
> interpretation like that does make sense to me. But I knew of no
> reason why you might find that style made the message offensive. It
> was never intended to denigrate.

This explains why it felt so difficult to make myself clear. I was
feeling exactly the same, just the other way round.

> I don't know you all that well, but we have talked for quite a while
> on a few occasions. I got the sense that you are a decent person from
> these conversations. I have no possible reason to denigrate or insult
> you. In general I try to be respectful, and if I ever fail it's not
> because I didn't care at all. Anybody that knows me well knows that I
> am not a mean spirited person.

I never though that. Maybe we should have quickly talked things out.
Email tends to be a bad medium for communication, especially when it
goes wrong. :)

> If this is just an unfortunate misunderstanding, as I suspect it is,
> then I would be happy to let it go, and treat it as something to
> learn
> from.

Agreed, me too. 

I'd like to apologize for not answering your email the way I should
have. Honestly it never occurred to me. Maybe that's because I'm used
to other procedures, or because I never had to converse with the RMT,
or maybe, quite simple, because I lacked the time to think it through,
the original issue that kind of started this whole mess.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-09T22:04:02Z

    On Mon, Aug  9, 2021 at 06:00:00PM -0400, Bruce Momjian wrote:
> > Again agreed, please keep in mind, though, that I didn't notice I was
> > being chased until Peter's first email.
> 
> I was asked by the RMT to contact one of your employees, and I did, to
> tell you that the RMT was looking for feedback from you on an ecpg
> issue.  Not sure if that was before or after Peter's email.

The date of that request was July 28 and I was told by your employee
that you would be informed that afternoon.  If you want the employee's
name, I will provide it in a private email.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-09T22:15:10Z

    On Tue, Aug 10, 2021 at 12:03:24AM +0200, Michael Meskes wrote:
> I'd like to apologize for not answering your email the way I should
> have. Honestly it never occurred to me. Maybe that's because I'm used
> to other procedures, or because I never had to converse with the RMT,
> or maybe, quite simple, because I lacked the time to think it through,
> the original issue that kind of started this whole mess.

Agreed.  When the RMT contacts me, I assume it is something that is time
and release critical so I give them as much detail as I can, and a
timeline when they will get more information.  If you are not focused on
the RMT process, it might not be clear why that is important.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-09T22:21:16Z

    > > Again agreed, please keep in mind, though, that I didn't notice I
> > was
> > being chased until Peter's first email.
> 
> I was asked by the RMT to contact one of your employees, and I did,
> to
> tell you that the RMT was looking for feedback from you on an ecpg
> issue.  Not sure if that was before or after Peter's email.

I think that was before, at that point I still thought it was nothing
time sensitive. And unfortunately it didn't register that RMT was
involved at all.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Andrew Dunstan <andrew@dunslane.net> — 2021-08-09T22:25:56Z

    On 8/9/21 6:15 PM, Bruce Momjian wrote:
> On Tue, Aug 10, 2021 at 12:03:24AM +0200, Michael Meskes wrote:
>> I'd like to apologize for not answering your email the way I should
>> have. Honestly it never occurred to me. Maybe that's because I'm used
>> to other procedures, or because I never had to converse with the RMT,
>> or maybe, quite simple, because I lacked the time to think it through,
>> the original issue that kind of started this whole mess.
> Agreed.  When the RMT contacts me, I assume it is something that is time
> and release critical so I give them as much detail as I can, and a
> timeline when they will get more information.  If you are not focused on
> the RMT process, it might not be clear why that is important.
>

That's what you should be doing. If nothing else comes from this
colloquy it should make all committers aware of the process. The reason
we have an RMT, as I understand it, is to prevent the situation we had
years ago when things sometimes dragged on almost interminably after
feature freeze.


cheers


andrew

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-09T22:42:45Z

    Michael,

On Mon, Aug 9, 2021 at 3:03 PM Michael Meskes <meskes@postgresql.org> wrote:
> This explains why it felt so difficult to make myself clear. I was
> feeling exactly the same, just the other way round.

My own special brand of miscommunication was also involved. I happen
to be sensitive to the perception that I yield any authority that I
might have (as an RMT member, as a committer, whatever) in a way that
is arbitrary or unfair. And so I wrote way too much about why that
wasn't actually the case here. I now realize that that wasn't really
relevant.

> I never though that. Maybe we should have quickly talked things out.
> Email tends to be a bad medium for communication, especially when it
> goes wrong. :)

Indeed. That might well have happened if we had been set up for it already.

> I'd like to apologize for not answering your email the way I should
> have. Honestly it never occurred to me. Maybe that's because I'm used
> to other procedures, or because I never had to converse with the RMT,
> or maybe, quite simple, because I lacked the time to think it through,
> the original issue that kind of started this whole mess.

I think that there was a snowballing effect here. We both made
mistakes that compounded. I apologize for my role in this whole mess.

-- 
Peter Geoghegan

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-09T22:51:36Z

    On Mon, Aug  9, 2021 at 03:42:45PM -0700, Peter Geoghegan wrote:
> > I'd like to apologize for not answering your email the way I should
> > have. Honestly it never occurred to me. Maybe that's because I'm used
> > to other procedures, or because I never had to converse with the RMT,
> > or maybe, quite simple, because I lacked the time to think it through,
> > the original issue that kind of started this whole mess.
> 
> I think that there was a snowballing effect here. We both made
> mistakes that compounded. I apologize for my role in this whole mess.

I do think all committers need to understand the role of the RMT so they
can respond appropriately.  Do we need to communicate this better?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-09T23:20:05Z

    On Mon, Aug 9, 2021 at 3:51 PM Bruce Momjian <bruce@momjian.us> wrote:
> > I think that there was a snowballing effect here. We both made
> > mistakes that compounded. I apologize for my role in this whole mess.
>
> I do think all committers need to understand the role of the RMT so they
> can respond appropriately.  Do we need to communicate this better?

I think that it makes sense to codify the practical expectations that
the community has of existing committers, at least to some degree. I
mean why wouldn't we? The resulting document (an addition to the
"committers" Postgres wiki page?) would inevitably leave certain
questions open to interpretation. That seems okay to me.

I don't feel qualified to write this myself. Just my opinion.

-- 
Peter Geoghegan

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-10T06:20:50Z

    On Mon, Aug 09, 2021 at 10:50:29PM +0200, Michael Meskes wrote:
>> On the substance of the issue, one question that I have reading over
>> this thread is whether there's really a bug here at all. It is being
>> represented (and I have not checked whether this is accurate) that
>> the
>> patch affects the behavior of  DECLARE, PREPARE, and EXECUTE, but not
>> DESCRIBE, DEALLOCATE, DECLARE CURSOR .. FOR, or CREATE TABLE AS
>> EXECUTE. However, it also seems that it's not entirely clear what the
>> behavior ought to be in those cases, except perhaps for DESCRIBE. If
>> that's the case, maybe this doesn't really need to be an open item,
>> and maybe we don't therefore need to talk about whether it should be
>> reverted. Maybe it's just a feature that supports certain things now
>> and in the future, after due reflection, perhaps it will support
>> more.
> 
> The way I see it we should commit the patch that makes more statements
> honor the new DECLARE STATEMENT feature. I don't think we can change
> anything for the worse by doing that. And other databases are not
> really strict about this either.

Okay.  So you mean to change DESCRIBE and DEALLOCATE to be able to
handle cached connection names, as of [1]?  For [DE]ALLOCATE, I agree
that it could be a good thing.  declare.pgc seems to rely on that
already but the tests are incorrect as I mentioned in [2].  For
DESCRIBE, that provides data about a result set, I find the assignment
of a connection a bit strange, and even if this would allow the use of
the same statement name for multiple connections, it seems to me that 
there is a risk of breaking existing applications.  There should not
be that many, so perhaps that's fine anyway.

[1]: https://www.postgresql.org/message-id/TYAPR01MB5866973462D17F2AEBD8EBB8F51F9@TYAPR01MB5866.jpnprd01.prod.outlook.com
[2]: https://www.postgresql.org/message-id/YOQNCyfxp868zZUV@paquier.xyz
--
Michael

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-10T07:26:31Z

    
Peter,

> I think that there was a snowballing effect here. We both made
> mistakes that compounded. I apologize for my role in this whole mess.

Completely agreed. I think we both took things for granted that the
other one didn't take into account at all. I'm sorry about that, too.

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-10T07:31:37Z

    > Okay.  So you mean to change DESCRIBE and DEALLOCATE to be able to
> handle cached connection names, as of [1]?  For [DE]ALLOCATE, I agree

Yes, at least technically. I think DESCRIBE should accept the cached
connection name, although it won't really matter.

> that it could be a good thing.  declare.pgc seems to rely on that
> already but the tests are incorrect as I mentioned in [2].  For
> DESCRIBE, that provides data about a result set, I find the
> assignment
> of a connection a bit strange, and even if this would allow the use
> of
> the same statement name for multiple connections, it seems to me that
> there is a risk of breaking existing applications.  There should not
> be that many, so perhaps that's fine anyway.

I don't think we'd break anything given that DECLARE STATEMENT is new.
Also please keep in mind that you can use EXEC SQL AT ... DESCRIBE ...;
already anyway. Again, not very meaningful but why should we accept a
connection one way but not the other?

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-10T07:37:19Z

    > I do think all committers need to understand the role of the RMT so
> they
> can respond appropriately.  Do we need to communicate this better?

Being the one who assumed a different procedure, yes please. :)

Thanks,
Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-08-10T09:27:46Z

    Dear Meskes and any hackers,

> Yes, at least technically. I think DESCRIBE should accept the cached
> connection name, although it won't really matter.

I sought docs too and I found that Pro*C have such a same policy,
so it might be reasonable:

https://docs.oracle.com/en/database/oracle/oracle-database/21/lnpcc/Oracle-dynamic-SQL.html#GUID-0EB50EB7-D4C8-401D-AFCD-340D281711C4



Anyway I revised patches again in the current spec. I separated them into 6 parts:

0001: move "connection = NULL" to top rule. This is per Wang.
0002: adds supporting deallocate statement.
0003: adds supporting describe statement. The above and this are main parts.
0004: adds warning then the connection is overwritten. This is per Horiguchi-san.
0005: adds warning then the connection is overwritten. This is per Horiguchi-san and Paquier.
0006: adds some tests.

0001 is the solution of follows:
https://www.postgresql.org/message-id/OSBPR01MB42141999ED8EFDD4D8FDA42CF2E29%40OSBPR01MB4214.jpnprd01.prod.outlook.com

This bug is caused because "connection = NULL" is missing is missing in some cases, so I force to
substitute NULL in the statement: rule, the top-level in the parse tree.
I also remove the substitution from output.c because such line is overlapped.
If you think this change is too large, I can erase 0001 and add a substitution to the end part of
ECPGCursorStmt rule. That approach is also resolve the bug and impact is very small.

0004 is an optional patch, this is not related with DEALLOCATE and DESCRIBE.
We were discussing about how should work when followings are pre-compiled and executed:

> EXEC SQL AT conn1 DECLARE stmt STATEMENT;
> EXEC SQL AT conn2 EXECUTE stmt INTO ..;

Currently line 2 will execute at conn1 without any warnings (and this is the Oracle's spec) but Horiguchi-san says it is non-obvious.
So I added a precompiler-warning when the above situation.
More discussion might be needed here, but this is not main part.

About 0005, see previous discussion:

> Since we don't allow descriptors with the same name even if they are
> for the different connections, I think we can set the current
> connection if any (which is set either by AT option or statement-bound
> one) to i->connection silently if i->connection is NULL in
> lookup_descriptor(). What do you think about this?

How do you think?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-10T17:18:23Z

    On Tue, Aug 10, 2021 at 09:37:19AM +0200, Michael Meskes wrote:
> > I do think all committers need to understand the role of the RMT so
> > they
> > can respond appropriately.  Do we need to communicate this better?
> 
> Being the one who assumed a different procedure, yes please. :)

I think my point was that committers should be required to understand
the RMT process, and if we need to communicate that better, let's do
that.  I don't think it should be the responsibility of RMT members to
communicate the RMT process every time they communicate with someone,
unless someone asks.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-10T18:05:29Z

    > I think my point was that committers should be required to understand
> the RMT process, and if we need to communicate that better, let's do
> that.  I don't think it should be the responsibility of RMT members
> to
> communicate the RMT process every time they communicate with someone,
> unless someone asks.

Completely agreed, my point was that documenting the process to some
extend would be helpful. For obvious reasons I'm the wrong person to do
that, though. :)

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Bruce Momjian <bruce@momjian.us> — 2021-08-10T18:07:00Z

    On Tue, Aug 10, 2021 at 08:05:29PM +0200, Michael Meskes wrote:
> > I think my point was that committers should be required to understand
> > the RMT process, and if we need to communicate that better, let's do
> > that.  I don't think it should be the responsibility of RMT members
> > to
> > communicate the RMT process every time they communicate with someone,
> > unless someone asks.
> 
> Completely agreed, my point was that documenting the process to some
> extend would be helpful. For obvious reasons I'm the wrong person to do
> that, though. :)

Agreed.  How is this, which already exists?

	https://wiki.postgresql.org/wiki/Release_Management_Team

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-10T18:16:49Z

    > Agreed.  How is this, which already exists?
> 
>         https://wiki.postgresql.org/wiki/Release_Management_Team

That I know, but I don't think it covers the issues we, or I, had up-
thread. Or do I miss something?

Speaking of RMT, Andrew, Michael, Peter, will you make the final
decision whether we commit Kuroda-san's patches?

They are fine by me. Another pair of eyes would be nice, though. maybe
you could have another look, Horiguchi-san?

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Andrew Dunstan <andrew@dunslane.net> — 2021-08-10T20:46:18Z

    On 8/10/21 2:16 PM, Michael Meskes wrote:
>> Agreed.  How is this, which already exists?
>>
>>         https://wiki.postgresql.org/wiki/Release_Management_Team
> That I know, but I don't think it covers the issues we, or I, had up-
> thread. Or do I miss something?


No, you're right, although I think it's implied. Maybe we need a
statement along these lines:


Committers are responsible for the resolution of open items that relate
to commits they have made. Action needs to be taken in a timely fashion,
and if there is any substantial delay in dealing with an item the
committer should provide a date by which they expect action to be
completed. The RMT will follow up where these requirements are not being
complied with.



>
> Speaking of RMT, Andrew, Michael, Peter, will you make the final
> decision whether we commit Kuroda-san's patches?
>
> They are fine by me. Another pair of eyes would be nice, though. maybe
> you could have another look, Horiguchi-san?
>


If they are fine by you then I accept that. After all, the reason we
want you to deal with this is not only that you made the original commit
but because you're the expert in this area.



cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

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

    Peter Geoghegan <pg@bowt.ie> — 2021-08-10T21:16:57Z

    On Tue, Aug 10, 2021 at 1:46 PM Andrew Dunstan <andrew@dunslane.net> wrote:
> No, you're right, although I think it's implied. Maybe we need a
> statement along these lines:

I agree with that, but to me it's more in the scope of what is
expected of committers in general. At a very high level. So it's not
something that I'd expect to see on the RMT Postgres Wiki page. I
would expect to see it on the committers Wiki page, somewhere like
that.

> If they are fine by you then I accept that. After all, the reason we
> want you to deal with this is not only that you made the original commit
> but because you're the expert in this area.

+1.

Nobody questioned the original commit, so it would be premature (if
not totally arbitrary) to change our approach now, at the first sign
of trouble. To the best of my knowledge there is no special risk with
applying this patch to address the behavioral inconsistencies, nor is
there any known special risk with any other fix. Including even
deciding to *not* fix the inconsistency in Postgres 14 based on
practical considerations -- for all I know Michael might be perfectly
justified in interpreting the patch as new feature work that's out of
scope now.

I don't feel qualified to even offer an opinion.

-- 
Peter Geoghegan

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-11T00:29:23Z

    On Tue, Aug 10, 2021 at 09:31:37AM +0200, Michael Meskes wrote:
>> that it could be a good thing.  declare.pgc seems to rely on that
>> already but the tests are incorrect as I mentioned in [2].  For
>> DESCRIBE, that provides data about a result set, I find the
>> assignment
>> of a connection a bit strange, and even if this would allow the use
>> of
>> the same statement name for multiple connections, it seems to me that
>> there is a risk of breaking existing applications.  There should not
>> be that many, so perhaps that's fine anyway.
> 
> I don't think we'd break anything given that DECLARE STATEMENT is new.

Sure, DECLARE does not matter as it is new.  However, please note that
the specific point I was trying to make with my link [2] from upthread
is related to the use of cached connection names with DEALLOCATE, as
of this line in the new test declare.pgc:
    EXEC SQL DEALLOCATE PREPARE stmt_2;

And DEALLOCATE is far from being new.

> Also please keep in mind that you can use EXEC SQL AT ... DESCRIBE ...;
> already anyway. Again, not very meaningful but why should we accept a
> connection one way but not the other?

No objections to that.
--
Michael

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-11T20:41:59Z

    > Sure, DECLARE does not matter as it is new.  However, please note
> that
> the specific point I was trying to make with my link [2] from
> upthread
> is related to the use of cached connection names with DEALLOCATE, as
> of this line in the new test declare.pgc:
>     EXEC SQL DEALLOCATE PREPARE stmt_2;
> 
> And DEALLOCATE is far from being new.

I'm not sure I understand. Any usage of DECLARE STATEMENT makes the
file need the current version of ecpg anyway. On the other hand
DEALLOCATE did not change its behavior if no DECLARE STATEMENT was
issued, or what did I miss?

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-12T17:21:01Z

    > No, you're right, although I think it's implied. Maybe we need a
> statement along these lines:
> 
> 
> Committers are responsible for the resolution of open items that
> relate
> to commits they have made. Action needs to be taken in a timely
> fashion,
> and if there is any substantial delay in dealing with an item the
> committer should provide a date by which they expect action to be
> completed. The RMT will follow up where these requirements are not
> being
> complied with.

I think that would be helpful, yes.

> If they are fine by you then I accept that. After all, the reason we
> want you to deal with this is not only that you made the original
> commit
> but because you're the expert in this area.

I will commit the patch(es). Thanks.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-08-15T03:08:44Z

    Michael Meskes <meskes@postgresql.org> writes:
> I will commit the patch(es). Thanks.

This commit appears to be responsible for new noise on stderr
during check-world:

$ make -s check-world >/dev/null
declare.pgc:123: WARNING: connection "con2" is overwritten to "con1".
declare.pgc:124: WARNING: connection "con2" is overwritten to "con1".
declare.pgc:135: WARNING: connection "con2" is overwritten to "con1".

Please do something about that.

(1) There should be no output to stderr in the tests.  Why isn't this
message being caught and redirected to the normal test output file?

(2) This message is both unintelligible and grammatically incorrect.

			regards, tom lane

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-16T06:27:26Z

    On Sat, Aug 14, 2021 at 11:08:44PM -0400, Tom Lane wrote:
> Please do something about that.
> 
> (1) There should be no output to stderr in the tests.  Why isn't this
> message being caught and redirected to the normal test output file?

These are generated during the compilation of the tests with the
pre-processor, so that's outside the test runs.

> (2) This message is both unintelligible and grammatically incorrect.

Yeah, debugging such tests would be more helpful if the name of the
DECLARE statement is included, at least.  Those messages being
generated is not normal anyway, which is something coming from the
tests as a typo with the connection name of stmt_3.

Michael, what do you think about the attached?
--
Michael

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-16T07:27:08Z

    On Wed, Aug 11, 2021 at 10:41:59PM +0200, Michael Meskes wrote:
> I'm not sure I understand. Any usage of DECLARE STATEMENT makes the
> file need the current version of ecpg anyway. On the other hand
> DEALLOCATE did not change its behavior if no DECLARE STATEMENT was
> issued, or what did I miss?

Yes, you are right here.  I went through the code again and noticed by
mistake.  Sorry for the noise.
--
Michael

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

    Michael Meskes <meskes@postgresql.org> — 2021-08-16T10:06:16Z

    > > (1) There should be no output to stderr in the tests.  Why isn't
> > this
> > message being caught and redirected to the normal test output file?
> 
> These are generated during the compilation of the tests with the
> pre-processor, so that's outside the test runs.

This is actually a deeper issue, we have no test for the compiler
itself, other than the source code it generates. We do not test
warnings or errors thrown by it. The topic has come up ages ago and we
simply removed the test that generated the (planned) warning message.

> > (2) This message is both unintelligible and grammatically
> > incorrect.
> 
> Yeah, debugging such tests would be more helpful if the name of the
> DECLARE statement is included, at least.  Those messages being
> generated is not normal anyway, which is something coming from the
> tests as a typo with the connection name of stmt_3.
> 
> Michael, what do you think about the attached?

I think what Tom was saying is that it should be either "is overwritten
with" or "is rewritten to", but you raise a very good point. Adding the
statement name makes the message better. I fully agree. However, it
should be the other way round, the DECLARE STATEMENT changes the
connection that is used. 

You patch removes the warning but by doing that also removes the
feature that is being tested.

I'm not sure what's the best way to go about it, Shall we accept to not
test this particular feature and remove the warning? After all this is
not the way the statement should be used, hence the warning. Or should
be keep it in and redirect the warning? In that case, we would also
lose other warnings that are not planned, though.

Any comments?

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-17T06:34:28Z

    On Mon, Aug 16, 2021 at 12:06:16PM +0200, Michael Meskes wrote:
> You patch removes the warning but by doing that also removes the
> feature that is being tested.

Oops.  If kept this way, this test scenario is going to need a comment
to explain exactly that.

> I'm not sure what's the best way to go about it, Shall we accept to not
> test this particular feature and remove the warning? After all this is
> not the way the statement should be used, hence the warning. Or should
> be keep it in and redirect the warning? In that case, we would also
> lose other warnings that are not planned, though.

FWIW, I would tend to drop the warning here.  I am not sure that this
is a use case interesting enough.  My 2c.
--
Michael

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-18T01:17:37Z

    On Tue, Aug 17, 2021 at 03:34:28PM +0900, Michael Paquier wrote:
> On Mon, Aug 16, 2021 at 12:06:16PM +0200, Michael Meskes wrote:
>> You patch removes the warning but by doing that also removes the
>> feature that is being tested.
> 
> Oops.  If kept this way, this test scenario is going to need a comment
> to explain exactly that.

Michael has adjusted that as of f576de1, so I am closing this open
item.
--
Michael

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

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2021-08-19T11:27:17Z

    Hi,

Sorry for being late. I had a vaccination.

I'm not sure about the rule that stderr should be removed
even if the pre-compiling state, but anyway I agree that
the warned case is not expected.
The wrong message is perfectly fault...

I confirmed your commit and I think it's OK. Thanks!

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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

    wangsh.fnst@fujitsu.com <wangsh.fnst@fujitsu.com> — 2021-08-25T05:10:57Z

    Hi, 

I find in ecpg.header, the source:
>			if (connection)
>			if (connection && strcmp(ptr->connection, connection) != 0)

The first if statement is useless. And in fix-ecpg-tests.patch:
>-			if (connection)
>-				mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten to %s.", connection, ptr->connection);
>+			if (connection && strcmp(ptr->connection, connection) != 0)
>+				mmerror(PARSE_ERROR, ET_WARNING, "declare statement %s using connection %s overwritten to connection %s.",
>+					name, connection, ptr->connection);
The patch seems right.

Delete first if statement, patch attached.

Regards,
Shenhao Wang

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

    Michael Paquier <michael@paquier.xyz> — 2021-08-25T05:34:21Z

    On Wed, Aug 25, 2021 at 05:10:57AM +0000, wangsh.fnst@fujitsu.com wrote:
> Delete first if statement, patch attached.

Indeed, this looks like a mismerge.  I'll apply that in a bit.
Funnily, Coverity did not mention that.
--
Michael