Thread: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
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
Attachment
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
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
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
"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
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
Attachment
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
Attachment
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
Attachment
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
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
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
Attachment
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
Attachment
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
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
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
Attachment
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
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
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
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
> 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
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
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
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
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
> 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
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
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
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
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
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
> 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
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
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
> 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
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
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.
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.
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
> > > 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
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.
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
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.
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.
> > 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
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
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
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.
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
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
Attachment
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
> 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
Attachment
> 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
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 isnon-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
Attachment
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.
> 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
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.
> 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
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
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
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
Attachment
> 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
Attachment
> 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
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
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
Attachment
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
Attachment
> > (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
Attachment
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
Attachment
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
Attachment
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
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
Attachment
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