ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

Started by Hayato Kuroda (Fujitsu)almost 5 years ago71 messageshackers
Jump to latest
#1Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com

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

Attachments:

v01-fix-deallocate-describe.patchapplication/octet-stream; name=v01-fix-deallocate-describe.patchDownload+300-246
#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Hayato Kuroda (Fujitsu) (#1)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#3Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Kyotaro Horiguchi (#2)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#4Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Kyotaro Horiguchi (#3)
RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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]https://github.com/postgres/postgres/blob/master/src/interfaces/ecpg/preproc/ecpg.trailer#L345, 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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Hayato Kuroda (Fujitsu) (#4)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

"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

#6Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Tom Lane (#5)
RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

Attachments:

v02-0001-fix-deallocate-describe.patchapplication/octet-stream; name=v02-0001-fix-deallocate-describe.patchDownload+33-25
v02-0002-add-test.patchapplication/octet-stream; name=v02-0002-add-test.patchDownload+192-25
#7Michael Paquier
michael@paquier.xyz
In reply to: Hayato Kuroda (Fujitsu) (#6)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#8Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#7)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#9Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Hayato Kuroda (Fujitsu) (#6)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#10Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Michael Paquier (#8)
RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#11Michael Paquier
michael@paquier.xyz
In reply to: Hayato Kuroda (Fujitsu) (#10)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#12Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Kyotaro Horiguchi (#9)
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

Attachments:

v03-0001-fix-deallocate-describe.patchapplication/octet-stream; name=v03-0001-fix-deallocate-describe.patchDownload+50-30
v03-0002-add-test.patchapplication/octet-stream; name=v03-0002-add-test.patchDownload+192-25
#13wangsh.fnst@fujitsu.com
wangsh.fnst@fujitsu.com
In reply to: Hayato Kuroda (Fujitsu) (#12)
RE: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#14Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Hayato Kuroda (Fujitsu) (#12)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#15Michael Meskes
meskes@postgresql.org
In reply to: Michael Paquier (#8)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

In reply to: Michael Meskes (#15)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

In reply to: Peter Geoghegan (#16)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#18Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#17)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

In reply to: Michael Meskes (#18)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#20Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#19)
Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

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

#21Andrew Dunstan
andrew@dunslane.net
In reply to: Michael Meskes (#20)
In reply to: Michael Meskes (#20)
#23Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#22)
In reply to: Michael Meskes (#23)
#25Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#24)
#26Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Michael Meskes (#25)
#27Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: wangsh.fnst@fujitsu.com (#13)
#28Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Kyotaro Horiguchi (#14)
#29Michael Meskes
meskes@postgresql.org
In reply to: Hayato Kuroda (Fujitsu) (#26)
In reply to: Michael Meskes (#25)
#31Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#30)
In reply to: Michael Meskes (#31)
#33Robert Haas
robertmhaas@gmail.com
In reply to: Michael Meskes (#18)
#34Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#32)
#35Michael Meskes
meskes@postgresql.org
In reply to: Robert Haas (#33)
#36David G. Johnston
david.g.johnston@gmail.com
In reply to: Michael Meskes (#34)
#37Bruce Momjian
bruce@momjian.us
In reply to: Michael Meskes (#34)
In reply to: Michael Meskes (#34)
#39Michael Meskes
meskes@postgresql.org
In reply to: Bruce Momjian (#37)
#40Bruce Momjian
bruce@momjian.us
In reply to: Michael Meskes (#39)
#41Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#38)
#42Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#40)
#43Bruce Momjian
bruce@momjian.us
In reply to: Michael Meskes (#41)
#44Michael Meskes
meskes@postgresql.org
In reply to: Bruce Momjian (#40)
#45Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#43)
In reply to: Michael Meskes (#41)
#47Bruce Momjian
bruce@momjian.us
In reply to: Peter Geoghegan (#46)
In reply to: Bruce Momjian (#47)
#49Michael Paquier
michael@paquier.xyz
In reply to: Michael Meskes (#35)
#50Michael Meskes
meskes@postgresql.org
In reply to: Peter Geoghegan (#46)
#51Michael Meskes
meskes@postgresql.org
In reply to: Michael Paquier (#49)
#52Michael Meskes
meskes@postgresql.org
In reply to: Bruce Momjian (#47)
#53Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Michael Meskes (#51)
#54Bruce Momjian
bruce@momjian.us
In reply to: Michael Meskes (#52)
#55Michael Meskes
meskes@postgresql.org
In reply to: Bruce Momjian (#54)
#56Bruce Momjian
bruce@momjian.us
In reply to: Michael Meskes (#55)
#57Michael Meskes
meskes@postgresql.org
In reply to: Bruce Momjian (#56)
#58Andrew Dunstan
andrew@dunslane.net
In reply to: Michael Meskes (#57)
In reply to: Andrew Dunstan (#58)
#60Michael Paquier
michael@paquier.xyz
In reply to: Michael Meskes (#51)
#61Michael Meskes
meskes@postgresql.org
In reply to: Michael Paquier (#60)
#62Michael Meskes
meskes@postgresql.org
In reply to: Andrew Dunstan (#58)
#63Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Meskes (#62)
#64Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#63)
#65Michael Paquier
michael@paquier.xyz
In reply to: Michael Meskes (#61)
#66Michael Meskes
meskes@postgresql.org
In reply to: Michael Paquier (#64)
#67Michael Paquier
michael@paquier.xyz
In reply to: Michael Meskes (#66)
#68Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#67)
#69Hayato Kuroda (Fujitsu)
kuroda.hayato@fujitsu.com
In reply to: Michael Paquier (#68)
#70wangsh.fnst@fujitsu.com
wangsh.fnst@fujitsu.com
In reply to: Hayato Kuroda (Fujitsu) (#69)
#71Michael Paquier
michael@paquier.xyz
In reply to: wangsh.fnst@fujitsu.com (#70)