Is there a complete doc to describe pg's traction implementation in detail?

[jacktby jacktby]

Re: Include the dependent extension information in describe command.

[vignesh C]

On Tue, Aug 16, 2022 at 9:04 PM Bruce Momjian  wrote:
>
> On Mon, Aug 15, 2022 at 10:09:29PM +0530, vignesh C wrote:
> > I have updated the patch to display "Objects depending on extension"
> > as describe extension footer. The changes for the same are available
> > in the v2 version patch attached. Thoughts?
>
> I wonder if we would be better off with a backslash command that showed
> the dependencies of any object.

Yes, If we have a backslash command which could show the dependencies
of the specified object could be helpful.
Can we something like below:
a) Index idx1 depend on table t1
create table t1(c1 int);
create index idx1 on t1(c1);
postgres=# \dD idx1
Name
-
idx1
Depends on:
table t1

b) Index idx1 depend on table t1 and extension ext1
alter index idx idx1 depends on extension ext1
postgres=# \dD idx1
Name
-
idx1
Depends on:
table t1
extension ext1

c) materialized view mv1 depends on table t1
create materialized view mv1 as select  * from t1;
postgres=# \dD mv1
Name
-
mv1
Depends on:
table t1

If you are ok with this approach, I can implement a patch on similar
lines. Thoughts?

Regards,
Vignesh

Re: Include the dependent extension information in describe command.

[Bruce Momjian]

On Mon, Aug 15, 2022 at 10:09:29PM +0530, vignesh C wrote:
> I have updated the patch to display "Objects depending on extension"
> as describe extension footer. The changes for the same are available
> in the v2 version patch attached. Thoughts?

I wonder if we would be better off with a backslash command that showed
the dependencies of any object.

-- 
  Bruce Momjian  https://momjian.us
  EDB  https://enterprisedb.com

  Indecision is a decision.  Inaction is an action.  Mark Batterson

Re: Include the dependent extension information in describe command.

[vignesh C]

On Sun, Aug 14, 2022 at 10:24 PM vignesh C  wrote:
>
> On Sun, Aug 14, 2022 at 11:07 AM Tom Lane  wrote:
> >
> > vignesh C  writes:
> > > Currently we do not include the dependent extension information for
> > > index and materialized view in the describe command. I felt it would
> > > be useful to include this information as part of the describe command
> > > like:
> > > \d+ idx_depends
> > >   Index "public.idx_depends"
> > >  Column |  Type   | Key? | Definition | Storage | Stats target
> > > +-+--++-+--
> > >  a  | integer | yes  | a  | plain   |
> > > btree, for table "public.tbl_idx_depends"
> > > Depends:
> > > "plpgsql"
> >
> > > Attached a patch for the same. Thoughts?
> >
> > This seems pretty much useless noise to me.  Can you point to
> > any previous requests for such a feature?  If we did do it,
> > why would we do it in such a narrow fashion (ie, only dependencies
> > of two specific kinds of objects on one other specific kind of
> > object)?  Why did you do it in this direction rather than
> > the other one, ie show dependencies when examining the extension?
>
> While implementing logical replication of "index which depends on
> extension", I found that this information was not available in any of
> the \d describe commands. I felt having this information in the \d
> describe command will be useful in validating the "depends on
> extension" easily. Now that you pointed out, I agree that it will be
> better to show the dependencies from the extension instead of handling
> it in multiple places. I will change it to handle it from extension
> and post an updated version soon for this.

I have updated the patch to display "Objects depending on extension"
as describe extension footer. The changes for the same are available
in the v2 version patch attached. Thoughts?

Regards,
Vignesh
From 3bdb382f38b889b303f7a7036d9a0fc5dbeb2be7 Mon Sep 17 00:00:00 2001
From: Vigneshwaran C 
Date: Thu, 28 Jul 2022 22:19:00 +0530
Subject: [PATCH v2] Include the objects depending on extension in describe
 extension

Include the objects depending on extension in describe extension
---
 .../expected/create_transform.out |  3 -
 src/bin/psql/describe.c   | 94 ---
 .../expected/test_extensions.out  |  6 --
 src/test/regress/expected/indexing.out| 17 
 src/test/regress/expected/matview.out | 16 
 src/test/regress/sql/indexing.sql |  7 ++
 src/test/regress/sql/matview.sql  |  6 ++
 7 files changed, 128 insertions(+), 21 deletions(-)

diff --git a/contrib/hstore_plperl/expected/create_transform.out b/contrib/hstore_plperl/expected/create_transform.out
index dc72395376..d060d6ff65 100644
--- a/contrib/hstore_plperl/expected/create_transform.out
+++ b/contrib/hstore_plperl/expected/create_transform.out
@@ -49,7 +49,6 @@ CREATE EXTENSION hstore_plperl;
  function hstore_to_plperl(internal)
  function plperl_to_hstore(internal)
  transform for hstore language plperl
-(3 rows)
 
 ALTER EXTENSION hstore_plperl DROP TRANSFORM FOR hstore LANGUAGE plperl;
 \dx+ hstore_plperl
@@ -58,7 +57,6 @@ Objects in extension "hstore_plperl"
 -
  function hstore_to_plperl(internal)
  function plperl_to_hstore(internal)
-(2 rows)
 
 ALTER EXTENSION hstore_plperl ADD TRANSFORM FOR hstore LANGUAGE plperl;
 \dx+ hstore_plperl
@@ -68,7 +66,6 @@ ALTER EXTENSION hstore_plperl ADD TRANSFORM FOR hstore LANGUAGE plperl;
  function hstore_to_plperl(internal)
  function plperl_to_hstore(internal)
  transform for hstore language plperl
-(3 rows)
 
 DROP EXTENSION hstore CASCADE;
 NOTICE:  drop cascades to extension hstore_plperl
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 327a69487b..003a3361ec 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -20,6 +20,7 @@
 #include "catalog/pg_cast_d.h"
 #include "catalog/pg_class_d.h"
 #include "catalog/pg_default_acl_d.h"
+#include "catalog/pg_extension_d.h"
 #include "common.h"
 #include "common/logging.h"
 #include "describe.h"
@@ -45,7 +46,12 @@ static bool describeOneTSConfig(const char *oid, const char *nspname,
 const char *cfgname,
 const char *pnspname, const char *prsname);
 static void printACLColumn(PQExpBuffer buf, const char *colname);
-static bool listOneExtensionContents(const char *extname, const char *oid);
+static bool listOneExtensionContents(const char *extname, const char *oid,
+	 printTableContent *const content,
+	 PQExpBufferData *title,
+	

Re: Include the dependent extension information in describe command.

[vignesh C]

On Sun, Aug 14, 2022 at 11:07 AM Tom Lane  wrote:
>
> vignesh C  writes:
> > Currently we do not include the dependent extension information for
> > index and materialized view in the describe command. I felt it would
> > be useful to include this information as part of the describe command
> > like:
> > \d+ idx_depends
> >   Index "public.idx_depends"
> >  Column |  Type   | Key? | Definition | Storage | Stats target
> > +-+--++-+--
> >  a  | integer | yes  | a  | plain   |
> > btree, for table "public.tbl_idx_depends"
> > Depends:
> > "plpgsql"
>
> > Attached a patch for the same. Thoughts?
>
> This seems pretty much useless noise to me.  Can you point to
> any previous requests for such a feature?  If we did do it,
> why would we do it in such a narrow fashion (ie, only dependencies
> of two specific kinds of objects on one other specific kind of
> object)?  Why did you do it in this direction rather than
> the other one, ie show dependencies when examining the extension?

While implementing logical replication of "index which depends on
extension", I found that this information was not available in any of
the \d describe commands. I felt having this information in the \d
describe command will be useful in validating the "depends on
extension" easily. Now that you pointed out, I agree that it will be
better to show the dependencies from the extension instead of handling
it in multiple places. I will change it to handle it from extension
and post an updated version soon for this.

Regards,
Vignesh

Re: Include the dependent extension information in describe command.

[Tom Lane]

vignesh C  writes:
> Currently we do not include the dependent extension information for
> index and materialized view in the describe command. I felt it would
> be useful to include this information as part of the describe command
> like:
> \d+ idx_depends
>   Index "public.idx_depends"
>  Column |  Type   | Key? | Definition | Storage | Stats target
> +-+--++-+--
>  a  | integer | yes  | a  | plain   |
> btree, for table "public.tbl_idx_depends"
> Depends:
> "plpgsql"

> Attached a patch for the same. Thoughts?

This seems pretty much useless noise to me.  Can you point to
any previous requests for such a feature?  If we did do it,
why would we do it in such a narrow fashion (ie, only dependencies
of two specific kinds of objects on one other specific kind of
object)?  Why did you do it in this direction rather than
the other one, ie show dependencies when examining the extension?

regards, tom lane

Include the dependent extension information in describe command.

[vignesh C]

Hi,

Currently we do not include the dependent extension information for
index and materialized view in the describe command. I felt it would
be useful to include this information as part of the describe command
like:
\d+ idx_depends
  Index "public.idx_depends"
 Column |  Type   | Key? | Definition | Storage | Stats target
+-+--++-+--
 a  | integer | yes  | a  | plain   |
btree, for table "public.tbl_idx_depends"
Depends:
"plpgsql"

Attached a patch for the same. Thoughts?

Regards,
Vignesh


v1-0001-Include-the-dependent-extension-information-for-i.patch
Description: Binary data

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

[Michael Paquier]

On Wed, Aug 25, 2021 at 05:10:57AM +, wangsh.f...@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


signature.asc
Description: PGP signature

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

[wangsh.f...@fujitsu.com]

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


0001-delete-dup-if-statement.patch
Description: 0001-delete-dup-if-statement.patch

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

[kuroda.hay...@fujitsu.com]

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

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

[Michael Paquier]

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


signature.asc
Description: PGP signature

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

[Michael Paquier]

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


signature.asc
Description: PGP signature

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

[Michael Meskes]

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


signature.asc
Description: This is a digitally signed message part

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

[Michael Paquier]

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


signature.asc
Description: PGP signature

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

[Michael Paquier]

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
diff --git a/src/interfaces/ecpg/preproc/ecpg.header b/src/interfaces/ecpg/preproc/ecpg.header
index 067c9cf8e7..863076945f 100644
--- a/src/interfaces/ecpg/preproc/ecpg.header
+++ b/src/interfaces/ecpg/preproc/ecpg.header
@@ -594,8 +594,9 @@ check_declared_list(const char *name)
 			continue;
 		if (strcmp(name, ptr -> name) == 0)
 		{
-			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);
 			connection = mm_strdup(ptr -> connection);
 			return true;
 		}
diff --git a/src/interfaces/ecpg/test/expected/sql-declare.c b/src/interfaces/ecpg/test/expected/sql-declare.c
index cff928204e..b9b189b656 100644
--- a/src/interfaces/ecpg/test/expected/sql-declare.c
+++ b/src/interfaces/ecpg/test/expected/sql-declare.c
@@ -374,7 +374,7 @@ if (sqlca.sqlcode < 0) sqlprint();}
 /* declare  \"stmt_3\"  as an SQL identifier */
 #line 122 "declare.pgc"
 
-{ ECPGprepare(__LINE__, "con1", 0, "stmt_3", selectString);
+{ ECPGprepare(__LINE__, "con2", 0, "stmt_3", selectString);
 #line 123 "declare.pgc"
 
 if (sqlca.sqlcode < 0) sqlprint();}
@@ -383,8 +383,8 @@ if (sqlca.sqlcode < 0) sqlprint();}
 /* declare cur_3 cursor for $1 */
 #line 124 "declare.pgc"
 
-{ ECPGdo(__LINE__, 0, 1, "con1", 0, ECPGst_normal, "declare cur_3 cursor for $1", 
-	ECPGt_char_variable,(ECPGprepared_statement("con1", "stmt_3", __LINE__)),(long)1,(long)1,(1)*sizeof(char), 
+{ ECPGdo(__LINE__, 0, 1, "con2", 0, ECPGst_normal, "declare cur_3 cursor for $1", 
+	ECPGt_char_variable,(ECPGprepared_statement("con2", "stmt_3", __LINE__)),(long)1,(long)1,(1)*sizeof(char), 
 	ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT, ECPGt_EORT);
 #line 125 "declare.pgc"
 
@@ -398,7 +398,7 @@ if (sqlca.sqlcode < 0) sqlprint();}
 i = 0;
 while (1)
 {
-{ ECPGdo(__LINE__, 0, 1, "con1", 0, ECPGst_normal, "fetch cur_3", ECPGt_EOIT, 
+{ ECPGdo(__LINE__, 0, 1, "con2", 0, ECPGst_normal, "fetch cur_3", ECPGt_EOIT, 
 	ECPGt_int,&(f1[i]),(long)1,(long)1,sizeof(int), 
 	ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, 
 	ECPGt_int,&(f2[i]),(long)1,(long)1,sizeof(int), 
@@ -415,13 +415,13 @@ if (sqlca.sqlcode < 0) sqlprint();}
 
 i++;
 }
-{ ECPGdo(__LINE__, 0, 1, "con1", 0, ECPGst_normal, "close cur_3", ECPGt_EOIT, ECPGt_EORT);
+{ ECPGdo(__LINE__, 0, 1, "con2", 0, ECPGst_normal, "close cur_3", ECPGt_EOIT, ECPGt_EORT);
 #line 134 "declare.pgc"
 
 if (sqlca.sqlcode < 0) sqlprint();}
 #line 134 "declare.pgc"
 
-{ ECPGdeallocate(__LINE__, 0, "con1", "stmt_3");
+{ ECPGdeallocate(__LINE__, 0, "con2", "stmt_3");
 #line 135 "declare.pgc"
 
 if (sqlca.sqlcode < 0) sqlprint();}
diff --git a/src/interfaces/ecpg/test/expected/sql-declare.stderr b/src/interfaces/ecpg/test/expected/sql-declare.stderr
index 29d0b828e7..65db974a85 100644
--- a/src/interfaces/ecpg/test/expected/sql-declare.stderr
+++ b/src/interfaces/ecpg/test/expected/sql-declare.stderr
@@ -142,13 +142,13 @@
 [NO_PID]: sqlca: code: 0, state: 0
 [NO_PID]: prepare_common on line 123: name stmt_3; query: "SELECT f1,f2,f3 FROM source"
 [NO_PID]: sqlca: code: 0, state: 0
-[NO_PID]: ecpg_execute on line 125: query: declare cur_3 cursor for SELECT f1,f2,f3 FROM source; with 0 parameter(s) on connection con1
+[NO_PID]: ecpg_execute on line 125: query: declare cur_3 cursor for SELECT f1,f2,f3 FROM source; with 0 parameter(s) on connection con2
 [NO_PID]: sqlca: code: 0, state: 0
 [NO_PID]: ecpg_execute on line 125: using PQexec
 [NO_PID]: sqlca: code: 0, state: 0
 [NO_PID]: ecpg_process_output on line 125: OK: DECLARE CURSOR
 [NO_PID]: sqlca: code: 0, state: 0
-[NO_PID]: ecpg_execute on line 131: query: fetch cur_3; with 0 parameter(s) on connection con1
+[NO_PID]: ecpg_execute on line 131: query: fetch cur_3; with 0 parameter(s) on connection con2
 [NO_PID]: sqlca: code: 0, state: 0
 [NO_PID]: ecpg_execute on line 131: using PQexec
 [NO_PID]: sqlca: code: 0, state: 0
@@ -158,9 +158,9 @@
 [NO_PID]: sqlca: code: 0, state: 0
 [NO_PID]: 

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

[Tom Lane]

Michael Meskes  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

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

[Michael Meskes]

> 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

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

[Michael Meskes]

> 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


signature.asc
Description: This is a digitally signed message part

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

[Michael Paquier]

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


signature.asc
Description: PGP signature

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

[Peter Geoghegan]

On Tue, Aug 10, 2021 at 1:46 PM Andrew Dunstan  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

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

[Andrew Dunstan]

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

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

[Michael Meskes]

> 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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[Michael Meskes]

> 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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[kuroda.hay...@fujitsu.com]

Dear Meskes and any hackers,

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

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

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



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

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

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

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

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

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

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

About 0005, see previous discussion:

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

How do you think?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED



v04-0001-move-toplevel.patch
Description: v04-0001-move-toplevel.patch


v04-0002-allow-deallocate.patch
Description: v04-0002-allow-deallocate.patch


v04-0003-allow-describe.patch
Description: v04-0003-allow-describe.patch


v04-0004-add-warning-in-check_declared_list.patch
Description: v04-0004-add-warning-in-check_declared_list.patch


v04-0005-descriptor.patch
Description: v04-0005-descriptor.patch


v04-0006-add-test.patch
Description: v04-0006-add-test.patch

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

[Michael Meskes]

> 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

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

[Michael Meskes]

> 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


signature.asc
Description: This is a digitally signed message part

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

[Michael Meskes]

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

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

[Michael Paquier]

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/tyapr01mb5866973462d17f2aebd8ebb8f5...@tyapr01mb5866.jpnprd01.prod.outlook.com
[2]: https://www.postgresql.org/message-id/yoqncyfxp868z...@paquier.xyz
--
Michael


signature.asc
Description: PGP signature

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

[Peter Geoghegan]

On Mon, Aug 9, 2021 at 3:51 PM Bruce Momjian  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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[Peter Geoghegan]

Michael,

On Mon, Aug 9, 2021 at 3:03 PM Michael Meskes  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

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

[Andrew Dunstan]

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

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

[Michael Meskes]

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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[Michael Meskes]

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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[Michael Meskes]

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

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

[Peter Geoghegan]

On Mon, Aug 9, 2021 at 1:38 PM Michael Meskes  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

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

[Bruce Momjian]

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  https://momjian.us
  EDB  https://enterprisedb.com

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

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

[David G. Johnston]

On Mon, Aug 9, 2021 at 1:38 PM Michael Meskes  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.

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

[Michael Meskes]

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

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

[Michael Meskes]

> 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

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

[Robert Haas]

On Sat, Aug 7, 2021 at 4:13 AM Michael Meskes  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

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

[Peter Geoghegan]

On Mon, Aug 9, 2021 at 11:45 AM Michael Meskes  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

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

[Michael Meskes]

> 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

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

[Peter Geoghegan]

On Mon, Aug 9, 2021 at 12:10 AM Michael Meskes  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

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

[Michael Meskes]

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

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

[kuroda.hay...@fujitsu.com]

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

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

[kuroda.hay...@fujitsu.com]

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

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

[kuroda.hay...@fujitsu.com]

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

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

[Michael Meskes]

> 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

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

[Peter Geoghegan]

On Sun, Aug 8, 2021 at 11:34 AM Michael Meskes  wrote:
> > https://postgr.es/m/CAH2-Wzk=qxtsp0h5ekv92eh0u22hvmqlhgwyp4_fa3ytiei...@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

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

[Michael Meskes]

On Sat, 2021-08-07 at 15:31 -0700, Peter Geoghegan wrote:
> On Sat, Aug 7, 2021 at 12:43 PM Michael Meskes
>  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_fa3ytiei...@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

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

[Peter Geoghegan]

On Sat, Aug 7, 2021 at 12:43 PM Michael Meskes  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_fa3ytiei...@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

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

[Andrew Dunstan]

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

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

[Michael Meskes]

> 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

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

[Peter Geoghegan]

On Sat, Aug 7, 2021 at 1:13 AM Michael Meskes  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

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

[Michael Meskes]

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

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

[Peter Geoghegan]

On Fri, Jul 30, 2021 at 3:01 PM Peter Geoghegan  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

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

[Peter Geoghegan]

On Thu, Jul 29, 2021 at 12:22 PM Michael Meskes  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

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

[Michael Meskes]

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


signature.asc
Description: This is a digitally signed message part

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

[Kyotaro Horiguchi]

Hello, Kuroda-san.

At Mon, 12 Jul 2021 04:05:21 +, "kuroda.hay...@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

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

[wangsh.f...@fujitsu.com]

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.hay...@fujitsu.com  
Sent: Monday, July 12, 2021 12:05 PM
To: 'Kyotaro Horiguchi' 
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

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

[kuroda.hay...@fujitsu.com]

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



v03-0001-fix-deallocate-describe.patch
Description: v03-0001-fix-deallocate-describe.patch


v03-0002-add-test.patch
Description: v03-0002-add-test.patch

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

[Michael Paquier]

On Thu, Jul 08, 2021 at 11:42:14AM +, kuroda.hay...@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


signature.asc
Description: PGP signature

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

[kuroda.hay...@fujitsu.com]

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

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

[Kyotaro Horiguchi]

At Fri, 2 Jul 2021 12:53:02 +, "kuroda.hay...@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

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

[Michael Paquier]

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


signature.asc
Description: PGP signature

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

[Michael Paquier]

On Fri, Jul 02, 2021 at 12:53:02PM +, kuroda.hay...@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: 0
[...]
-[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: 0

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


signature.asc
Description: PGP signature

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

[kuroda.hay...@fujitsu.com]

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



v02-0001-fix-deallocate-describe.patch
Description: v02-0001-fix-deallocate-describe.patch


v02-0002-add-test.patch
Description: v02-0002-add-test.patch

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

[Tom Lane]

"kuroda.hay...@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

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

[kuroda.hay...@fujitsu.com]

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

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

[Kyotaro Horiguchi]

At Thu, 01 Jul 2021 17:48:49 +0900 (JST), Kyotaro Horiguchi 
 wrote in 
> At Fri, 25 Jun 2021 12:02:22 +, "kuroda.hay...@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

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

[Kyotaro Horiguchi]

At Fri, 25 Jun 2021 12:02:22 +, "kuroda.hay...@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

ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

[kuroda.hay...@fujitsu.com]

Dear Hackers,

I checked about DECLARE STATEMENT(added from ad8305a), and I noticed that
this connection-control feature cannot be used for DEALLOCATE and DESCRIBE 
statement.

I attached the patch that fixes these bugs, this contains source and test code.

How do you think? 

Best Regards,
Hayato Kuroda
FUJITSU LIMITED



v01-fix-deallocate-describe.patch
Description: v01-fix-deallocate-describe.patch

Re: describe-config issue

[vignesh C]

On Wed, Sep 2, 2020 at 8:06 PM Tom Lane  wrote:

> Please defend that claim.  Otherwise this seems like a pretty
> random change.

I had seen that there is discrepancy in postgres --describe-config & the
value displayed from pg_settings like in the below case:
postgres=# select name,min_val, max_val, boot_val,reset_val from
pg_settings where name = 'checkpoint_timeout';
name| min_val | max_val | *boot_val | reset_val*
+-+-+--+---
 checkpoint_timeout | 30  | 86400   | *300  | 300*
(1 row)

[vignesh@localhost bin]$ ./postgres --describe-config | grep
checkpoint_timeout
checkpoint_timeout sighup Write-Ahead Log / Checkpoints INTEGER *0 *30
86400 Sets the maximum time between automatic WAL checkpoints.

In the case of pg_settings we display 300 for boot_val/reset_val whereas in
the case of describe-config we display 0, shouldn't it be 300 here?
Thoughts?

 Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

Re: describe-config issue

[Tom Lane]

vignesh C  writes:
> Postgres's describe-config option prints reset_val for int & real
> configuration parameters which is not useful as it is not updated.

Uh, what?

> Printing boot_val is better in this case.

Please defend that claim.  Otherwise this seems like a pretty
random change.

regards, tom lane

describe-config issue

[vignesh C]

Hi,

Postgres's describe-config option prints reset_val for int & real
configuration parameters which is not useful as it is not updated.
Printing boot_val is better in this case. reset_val is updated with
boot_val while the server is getting started but in case of postgres
--describe-config this value is not updated. I felt printing boot_val
is more appropriate in this case. Attached patch for the same.
Thoughts?

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
From 847406e95ed35b5ce77c407a4b1b990e42e49125 Mon Sep 17 00:00:00 2001
From: Vignesh C 
Date: Wed, 2 Sep 2020 10:06:06 +0530
Subject: [PATCH] describe-config issue

describe-config prints reset_val for int & real configuration parameters which
is not useful as it is not updated. Printing boot_val is better in this case.
---
 src/backend/utils/misc/help_config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/misc/help_config.c b/src/backend/utils/misc/help_config.c
index c0120e1..7e5338a 100644
--- a/src/backend/utils/misc/help_config.c
+++ b/src/backend/utils/misc/help_config.c
@@ -103,14 +103,14 @@ printMixedStruct(mixedStruct *structToPrint)
 
 		case PGC_INT:
 			printf("INTEGER\t%d\t%d\t%d\t",
-   structToPrint->integer.reset_val,
+   structToPrint->integer.boot_val,
    structToPrint->integer.min,
    structToPrint->integer.max);
 			break;
 
 		case PGC_REAL:
 			printf("REAL\t%g\t%g\t%g\t",
-   structToPrint->real.reset_val,
+   structToPrint->real.boot_val,
    structToPrint->real.min,
    structToPrint->real.max);
 			break;
-- 
1.8.3.1

Re: \describe*

[Alvaro Herrera]

On 2019-Aug-01, Corey Huinker wrote:

> From all this, I have so far concluded:
> 
> 1. There is real demand to be able to easily see the basic structure of
> tables, views, and indexes in a way that strikes a balance between detail
> and clutter.

That's great.  That said, I'm not opposed to a DESCRIBE server-side
command, and others have shown some interest too.  However, the thread
and commitfest entry at hand refer to a new psql command \describe,
which is completely unrelated.  So I suggest we should close this CF
entry as Returned with Feedback, and wait until Corey comes back with a
server-side patch for DESCRIBE.  I don't see the point of keeping a
\describe item alive if the patch we ultimately end up doing is
something completely different.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: \describe*

[Corey Huinker]

>
> It seems this topic is ongoing so I've moved it to the September CF,
> but it's in "Waiting on Author" because we don't have a concrete patch
> that applies (or agreement on what it should do?) right now.
>

All recent work has been investigating the need(s) we're trying to address.
This is as good of a time as any to share my findings (with much
collaboration with Dave Fetter) so far.

1. Adding helper commands to psql aids only psql, and a great number of
users do not, or can not, use psql. So adding something on the server side
would have broader usage and appeal. Furthermore, some access tools
(especially browser-based ones) are not good about returning non-tabular
results, so helper commands that return result sets would have the broadest
usage.

2. Our own interest in server-side commands is all over the map. Some just
want the convenience of having them server side, or familiarity with
$OTHER_DB. Others want to eliminate the need for some code in pg_dump,
JDBC, or elsewhere.

3. There isn't much consensus in the other databases, though all of them do
*something*:

SQLServer
---

SQLServer has sp_help (
https://docs.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-help-transact-sql?view=sql-server-2017
 )

which contextually returns one of two different result sets (name, owner,
object type) or (column name, type, storage, length, precision, scale,
nullable, default, rule, collation)

DB2
--
Has a describe command (source:
https://www.ibm.com/support/knowledgecenter/SSEPGG_11.1.0/com.ibm.db2.luw.admin.cmd.doc/doc/r0002019.html)
 which
can be used to describe query output (data type, data type length, column
name, column name length).

It also has an option to DESCRIBE TABLE foo which returns a set of
(col_name, schema_of_datatype, data_type, data_type_length,
data_type_scale, Nulls t/f)

It also has DESCRIBE INDEXES FOR TABLE foo which returns a set of (schema
of index, name of index, unique flag, number of columns, index type)

It also has DESCRIBE DATA PARTITIONS FOR TABLE which as you might guess
shows partitions.

All of these options have a SHOW DETAIL modifier which adds more columns.

MySQL
--

(https://dev.mysql.com/doc/refman/8.0/en/show-columns.html)
MySSQL has SHOW COLUMNS which also returns a set of  (name, type similar to
format_type(), null flag, PK or index indicator, default value, notes about
auto-increment/autogreneration/implicit trggers), and can be extended to
show privileges and comments with the EXTENDED and FULL options.

MySQL has a DESCRIBE command, but it is a synonym of EXPLAIN.

MySQL also has a raft of commands like SHOW CREATE USER, SHOW CREATE VIEW,
SHOW CREATE TRIGGER, SHOW CREATE TABLE, etc. (ex:
https://dev.mysql.com/doc/refman/8.0/en/show-create-user.html)  These
commands all return a result set of of exactly one column, each row
representing one SQL statement, essentially doing a single-object
schema-only pg_dump.

Oracle
-

https://docs.oracle.com/cd/B19306_01/server.102/b14357/ch12019.htm

SQL*Plus has a describe command that works on tables and views and
composite types (tabular set of: name, null, type) procedures (tabular set
of: arg name, type, in/out), and packages (a series of sets one per type
and procedure)

SQLcl has the INFO statement, which is roughly analogous to psql's \d in
that it is a mix of tabular and non-tabular information.

Oracle itself has dbms_metadata.get_ddl() which seems analogous to mysql's
SHOW CREATE commands.

Snowflake
--

Snowflake has DESCRIBE TABLE
https://docs.snowflake.net/manuals/sql-reference/sql/desc-table.html and
DESCRIBE VIEW
https://docs.snowflake.net/manuals/sql-reference/functions/get_ddl.html

Which return a set of: (name, type, column type, null flag, default,
primary key, unique key, check, expression, comment).

It also has an option for describing "stage" tables, which are s3 buckets
with a file format associated, the closest postgresql analog would be a
file_fdw foreign table, and there is a separate result set format for that.

Snowflake has no concept of indexes (it understands that there's things
called a unique keys, and it remembers that you said you wanted one, but
does nothing to enforce it), so no command for that.

These result sets are not composable in a query, however, they are stored
in the RESULT_SCAN cache, which means that you can run a describe, and then
immediately fetch the results of that command as if it was a table.

Snowflake also has a get_ddl() function
https://docs.snowflake.net/manuals/sql-reference/sql/desc-view.html which
is a one-column result set of statements to re-create the given object.


>From all this, I have so far concluded:

1. There is real demand to be able to easily see the basic structure of
tables, views, and indexes in a way that strikes a balance between detail
and clutter.
2. There is some acknowledgement that this data be useful if it was further
filter

Re: \describe*

[Thomas Munro]

On Sun, Jun 23, 2019 at 7:34 AM Corey Huinker  wrote:
>> > So what is the uptake on implementing this at the server side, ie.
>> > DESCRIBE?
>>
>> I'm pretty skeptical of this idea, unless you are willing to throw
>> away at least one and possibly both of the following goals:

It seems this topic is ongoing so I've moved it to the September CF,
but it's in "Waiting on Author" because we don't have a concrete patch
that applies (or agreement on what it should do?) right now.

-- 
Thomas Munro
https://enterprisedb.com

Re: \describe*

[Corey Huinker]

>
> > So what is the uptake on implementing this at the server side, ie.
> > DESCRIBE?
>
> I'm pretty skeptical of this idea, unless you are willing to throw
> away at least one and possibly both of the following goals:
>
> 1. Compatibility with psql's existing \d behavior.
>

I don't think *compatibility* with the behavior should be a goal in itself.
Coverage of the majority of the use-cases is.

2. Usability of DESCRIBE for any purpose whatsoever other than emitting
> something that looks just like what psql prints.
>
> We've migrated many of the \d displays so far away from "a single query
> result" that I don't believe there's a way for a server command to
> duplicate them, at least not without some seriously unholy in-bed-ness
> between the server command and some postprocessing logic in describe.c.
> (At which point you've lost whatever system architectural value there
> might be in the whole project, since having a more-arm's-length
> relationship there kinda seems like the point to me.)
>

I think there's a genuine use for regular printed output, and there's also
a use for a query-able output. Maybe that queryable output is just a JSONB
output that the outer query can pick apart as it sees fit, and that would
handle the fact that the data often doesn't fit into a single query's
output.

Incidentally, I had need of this very functionality in Snowflake the other
day. The data dictionary there isn't capable of telling you which columns
are in a primary key, but that information is printed when you run
"DESCRIBE my_table".  The workaround is to run "DESCRIBE my_table" and then
make another query using a table function to recall the output of the last
query made in the session, and then filter that. Yeah, as a pattern it's
weird and sad, but it shows that there's are uses for something
DESCRIBE-ish on the server side.

So if we're going servier-side on DESCRIBE, it should be it's own entity,
not beholden to design decisions made in psql.


> There are a bunch of other little behavioral differences that you just
> can't replicate server-side, like the fact that localization of the
> results depends on psql's LC_MESSAGES not the server's.  Maybe people
> would be okay with changing that, but it's not a transparent
> reimplementation.
>

I think people would be OK with that. We're asking the server what it knows
about an object, not how psql feels about that same information.

I think if we want to have server-side describe capability, we're better
> off just to implement a DESCRIBE command that's not intended to be exactly
> like \d anything, and not try to make it be the implementation for \d
> anything.  (This was, in fact, where David started IIUC.  Other people's
> sniping at that idea hasn't yielded any better idea.)
>

I'm very much in support of server-side DESCRIBE that's not beholden to \d
in any way. For instance, I'm totally fine with DESCRIBE not being able to
handle wildcard patterns.

My initial suggestion for client-side \describe was mostly borne of it
being easy to implement a large subset of the \d commands to help users.
Not all users have psql access, so having a server side command helps more
people.

It could be that we decide that DESCRIBE is set-returning, and we have to
break up \d functionality to suit. By this I mean that we might find it
simpler to require DESCRIBE TABLE foo to only show columns with minimal
information about PKs and follow up commands like "DESCRIBE TABLE foo
INDEXES" or "DESCRIBE TABLE foo CONSTRAINTS" to keep output in tabular
format.


> In particular, I'm really strongly against having "\describe-foo-bar"
> invoke DESCRIBE, because (a) that will break compatibility with the
> existing \des command, and (b) it's not actually saving any typing,
> and (c) I think it'd confuse users no end.
>

+1. Having psql figure out which servers can give proper
servier-side-describes would boggle the mind.


> Of course, this line of thought does lead to the conclusion that we'd be
> maintaining psql/describe.c and server-side DESCRIBE in parallel forever,
>

Not fun, but what's our motivation for adding new new \d functionality once
a viable DESCRIBE is in place? Wouldn't the \d commands essentially be
feature-frozen at that point?


> which doesn't sound like fun.  But we should be making DESCRIBE with an
> eye to more use-cases than psql.  If it allows jdbc to not also maintain
> a pile of equivalent code, that'd be a win.  If it allows pg_dump to toss
> a bunch of logic overboard (or at least stop incrementally adding new
> variants), that'd be a big win.
>

I don't know enough about JDBC internals to know what sort of non-set
results it can handle, but that seems key to showing us how to proceed.

As for pg_dump, that same goal was a motivation for a similar server-side
command "SHOW CREATE " (essentially, pg_dump of ) which
would have basically the same design issues as DESCRIBE would, though the
result set would be a much simpler SETOF text.

Re: \describe*

[David Fetter]

On Fri, Jun 21, 2019 at 05:49:43PM -0400, Alvaro Herrera wrote:
> On 2019-Jun-21, David Fetter wrote:
> 
> > On Fri, Jun 21, 2019 at 05:20:54PM -0400, Alvaro Herrera wrote:
> > > On 2018-Jan-29, David Fetter wrote:
> > > 
> > > > We could certainly have \d call DESCRIBE for later versions of the
> > > > server.  \ commands which call different SQL depending on server
> > > > version have long been a standard practice.
> > > 
> > > So what is the uptake on implementing this at the server side, ie.
> > > DESCRIBE?
> > 
> > I've got a few Round Tuits available this weekend.  This seems like a
> > worthwhile thing to spend them on.
> 
> That's great, but my question is whether you managed to convince anyone
> whether it's a good idea.

Everybody who's used MySQL will.  In some sense, I'm more concerned
about the users in the future, who I hope vastly outnumber the users
in the present and past.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: \describe*

[Tom Lane]

Alvaro Herrera  writes:
> On 2018-Jan-29, David Fetter wrote:
>> We could certainly have \d call DESCRIBE for later versions of the
>> server.  \ commands which call different SQL depending on server
>> version have long been a standard practice.

> So what is the uptake on implementing this at the server side, ie.
> DESCRIBE?

I'm pretty skeptical of this idea, unless you are willing to throw
away at least one and possibly both of the following goals:

1. Compatibility with psql's existing \d behavior.

2. Usability of DESCRIBE for any purpose whatsoever other than emitting
something that looks just like what psql prints.

We've migrated many of the \d displays so far away from "a single query
result" that I don't believe there's a way for a server command to
duplicate them, at least not without some seriously unholy in-bed-ness
between the server command and some postprocessing logic in describe.c.
(At which point you've lost whatever system architectural value there
might be in the whole project, since having a more-arm's-length
relationship there kinda seems like the point to me.)

There are a bunch of other little behavioral differences that you just
can't replicate server-side, like the fact that localization of the
results depends on psql's LC_MESSAGES not the server's.  Maybe people
would be okay with changing that, but it's not a transparent
reimplementation.

I think if we want to have server-side describe capability, we're better
off just to implement a DESCRIBE command that's not intended to be exactly
like \d anything, and not try to make it be the implementation for \d
anything.  (This was, in fact, where David started IIUC.  Other people's
sniping at that idea hasn't yielded any better idea.)

In particular, I'm really strongly against having "\describe-foo-bar"
invoke DESCRIBE, because (a) that will break compatibility with the
existing \des command, and (b) it's not actually saving any typing,
and (c) I think it'd confuse users no end.

Of course, this line of thought does lead to the conclusion that we'd be
maintaining psql/describe.c and server-side DESCRIBE in parallel forever,
which doesn't sound like fun.  But we should be making DESCRIBE with an
eye to more use-cases than psql.  If it allows jdbc to not also maintain
a pile of equivalent code, that'd be a win.  If it allows pg_dump to toss
a bunch of logic overboard (or at least stop incrementally adding new
variants), that'd be a big win.

regards, tom lane

Re: \describe*

[Alvaro Herrera]

On 2019-Jun-21, David Fetter wrote:

> On Fri, Jun 21, 2019 at 05:20:54PM -0400, Alvaro Herrera wrote:
> > On 2018-Jan-29, David Fetter wrote:
> > 
> > > We could certainly have \d call DESCRIBE for later versions of the
> > > server.  \ commands which call different SQL depending on server
> > > version have long been a standard practice.
> > 
> > So what is the uptake on implementing this at the server side, ie.
> > DESCRIBE?
> 
> I've got a few Round Tuits available this weekend.  This seems like a
> worthwhile thing to spend them on.

That's great, but my question is whether you managed to convince anyone
whether it's a good idea.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: \describe*

[David Fetter]

On Fri, Jun 21, 2019 at 05:20:54PM -0400, Alvaro Herrera wrote:
> On 2018-Jan-29, David Fetter wrote:
> 
> > We could certainly have \d call DESCRIBE for later versions of the
> > server.  \ commands which call different SQL depending on server
> > version have long been a standard practice.
> 
> So what is the uptake on implementing this at the server side, ie.
> DESCRIBE?

I've got a few Round Tuits available this weekend.  This seems like a
worthwhile thing to spend them on.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: \describe*

[Alvaro Herrera]

On 2018-Jan-29, David Fetter wrote:

> We could certainly have \d call DESCRIBE for later versions of the
> server.  \ commands which call different SQL depending on server
> version have long been a standard practice.

So what is the uptake on implementing this at the server side, ie.
DESCRIBE?

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: describe working as intended?

[Melanie Plageman]

On Sat, May 18, 2019 at 1:17 AM Sergei Kornilov  wrote:

> Hello
>
> No, this is not bug. This is expected beharior of search_path setting:
> https://www.postgresql.org/docs/current/runtime-config-client.html
>
> > Likewise, the current session's temporary-table schema, pg_temp_nnn, is
> always searched if it exists. It can be explicitly listed in the path by
> using the alias pg_temp. If it is not listed in the path then it is
> searched first
>
> psql \d command checks current search_path (by pg_table_is_visible call).
> You can use \d *.t1 syntax to display tables with such name in all schemas.
>
> regards, Sergei
>


Thanks! I suppose it would behoove me to check the documentation
before resorting to looking at the source code :)

-- 
Melanie Plageman

Re: describe working as intended?

[Sergei Kornilov]

Hello

No, this is not bug. This is expected beharior of search_path setting: 
https://www.postgresql.org/docs/current/runtime-config-client.html

> Likewise, the current session's temporary-table schema, pg_temp_nnn, is 
> always searched if it exists. It can be explicitly listed in the path by 
> using the alias pg_temp. If it is not listed in the path then it is searched 
> first

psql \d command checks current search_path (by pg_table_is_visible call). You 
can use \d *.t1 syntax to display tables with such name in all schemas.

regards, Sergei

Re: describe working as intended?

[Tom Lane]

Melanie Plageman  writes:
> So, I noticed that if I make a table in one schema and then a table with the
> same name in another schema that describe only shows me one of them.

Yes, that's intended, psql's \d will only show you tables that are
visible in the search path, unless you give it a qualified pattern.
You can do something like "\d *.t1" if you want to see all the
instances of t1.

This is documented I believe ... ah yes, here:

Whenever the pattern parameter is omitted completely, the \d commands
display all objects that are visible in the current schema search path
— this is equivalent to using * as the pattern. (An object is said to
be visible if its containing schema is in the search path and no
object of the same kind and name appears earlier in the search
path. This is equivalent to the statement that the object can be
referenced by name without explicit schema qualification.) To see all
objects in the database regardless of visibility, use *.* as the
pattern.
...
A pattern that contains a dot (.) is interpreted as a schema name
pattern followed by an object name pattern. For example, \dt
foo*.*bar* displays all tables whose table name includes bar that are
in schemas whose schema name starts with foo. When no dot appears,
then the pattern matches only objects that are visible in the current
schema search path. Again, a dot within double quotes loses its
special meaning and is matched literally.

regards, tom lane

describe working as intended?

[Melanie Plageman]

So, I noticed that if I make a table in one schema and then a table with the
same name in another schema that describe only shows me one of them.
Demonstrating with temp table and regular table just for simplicity:
If I make a temp table t1 and a normal table t1 (it doesn't
matter which one I create first), describe only shows the temp table.

test=# create table t1();
CREATE TABLE
test=# \d
 List of relations
 Schema | Name | Type  |   Owner
+--+---+---
 public | t1   | table | mplageman
(1 row)

test=# create temp table t1();
CREATE TABLE
test=# \d
  List of relations
  Schema   | Name | Type  |   Owner
---+--+---+---
 pg_temp_4 | t1   | table | mplageman
(1 row)

I'm not sure if this is the intended behavior or if it is a bug.

I looked briefly at the describe code and ran the query in
describeTableDetails
which it constructs at the beginning and this, of course, returns the
results I
would expect.

test=# select c.oid, n.nspname, c.relname from pg_catalog.pg_class c left
join
pg_catalog.pg_namespace n on n.oid = c.relnamespace where c.relname = 't1';
  oid  |  nspname  | relname
---+---+-
 23609 | public| t1
 23612 | pg_temp_4 | t1
(2 rows)

So, without much more digging, is the current behavior of describe intended?
I couldn't find an email thread discussing this with the search terms I
tried.

(I noticed it on master and checked 11 as well and got the same behavior.)

-- 
Melanie Plageman

Re: \describe*

[Pavel Stehule]

Hi


> Since this is now waiting for v13, there's a bit more time to entertain
> the question of whether we'd rather have these in psql or in a new server
> command DESCRIBE [verbose] [system], and if so, whether the output of that
> would itself be query-able or not.
>

Including this feature in core can be nice. If they are on server side,
then should to produce result via API - like EXPLAIN. That's all.

Regards

Pavel

Re: \describe*

[Corey Huinker]

On Mon, Mar 4, 2019 at 1:45 PM Corey Huinker 
wrote:

>
>>> - Tab completion for \descibe-verbose.
>>> I know that \d+ tab completion is also not there, but I think we must
>>> have tab completion for \descibe-verbose.
>>>
>>> postgres=# \describe-
>>> \describe-extension
>>>  \describe-replication-publication     \describe-user-mapping
>>> \describe-foreign-data-wrapper
>>> \describe-replication-subscription\describe-view
>>> \describe-foreign-server  \describe-role
>>> \describe-window-function
>>> \describe-foreign-table   \describe-rule
>>>  ...
>>>
>>
> I just confirmed that there isn't tab completion for the existing S/+
> options, so it's hard to justify them for the equivalent verbose suffixes.
>

We can add completions for describe[-thing-]-verbose, but the
auto-completions start to run into combinatoric complexity, and the
original short-codes don't do that completion, probably for the same reason.

+   success =
>>> listTables("tvmsE", NULL, show_verbose, show_system);
>>> +   }
>>> +   status =
>>> PSQL_CMD_UNKNOWN;
>>>
>>>
> I'll look into this, thanks!
>

This was fixed, good find.



> - Confusion about \desc and \desC
>>> There is confusion while running the \desc command. I know the problem,
>>> but the user may confuse by this.
>>> postgres=# \desC
>>>List of foreign servers
>>>  Name | Owner | Foreign-data wrapper
>>> --+---+--
>>> (0 rows)
>>>
>>> postgres=# \desc
>>> Invalid command \desc. Try \? for help.
>>>
>>
I've changed the code to first strip out 0-1 instances of "-verbose" and
"-system" and the remaining string must be an exact match of a describe
command or it's an error. This same system could be applied to the short
commands to strip out 'S' and '+' and it might clean up the original code a
bit.

This command shows a list of relation "\d"
>>> postgres=# \describe-aggregatE-function
>>> List of relations
>>>  Schema | Name | Type  |  Owner
>>> +--+---+-----
>>>  public | foo  | table | vagrant
>>> (1 row)
>>>
>>
Same issue, same fix.


>>> I have done a brief code review except for the documentation code. I
>>> don't like this code
>>>
>>> if (cmd_match(cmd,"describe-aggregate-function"))
>>>
>>>  success = describeAggregates(pattern, show_verbose, show_system);
>>>  else if (cmd_match(cmd,
>>> "describe-access-method"))
>>>      success =
>>> describeAccessMethods(pattern, show_verbose);
>>>  else if (cmd_match(cmd,
>>> "describe-tablespace"))
>>>  success = describeTablespaces(pattern,
>>> show_verbose);
>>>  else if (cmd_match(cmd,
>>> "describe-conversion"))
>>>  success = listConversions(pattern,
>>> show_verbose, show_system);
>>>  else if (cmd_match(cmd, "describe-cast"))
>>>  success = listCasts(pattern,
>>> show_verbose
>>>
>>>
>>> This can be achieved with the list/array/hash table, so I have changed
>>> that code in the attached patch just for a sample if you want I can do that
>>> for whole code.
>>>
>>
> There's some problems with a hash table. The function signatures vary
> quite a lot, and some require additional psql_scan_slash_options to be
> called. The hash option, if implemented, probably should be expanded to all
> slash commands, at which point maybe it belongs in psqlscanslash.l...
>

As I suspected, there's a lot of variance in the function signatures of the
various listSomething()/describeSomething() commands,
and listDbRoleSettings requires a second pattern to be scanned, and as far
as I know PsqlScanState isn't known inside describe.h, so building and
using a hash table would be a lot of work for uncertain gain. The original
code just plows through strings in alphabetical order, breaking things up
by comparing leading characters, so I largely did the same at the
des/decribe levels.

Instead of a hash table, It might be fun to write

Re: Re: \describe*

[Corey Huinker]

>
>
> I agree with Andres and Robert.  This patch should be pushed to PG13.
>
> I'll do that on March 8 unless there is a compelling argument not to.
>
>
No objection. I'll continue to work on it, though.

Re: Re: \describe*

[David Steele]

On 2/25/19 9:44 PM, Robert Haas wrote:

On Sat, Feb 23, 2019 at 7:19 PM Andres Freund  wrote:

Sure, but it was late, and we have far more patches than we can deal
with. Many of them much much older than this.


More importantly, at least in my opinion, is that this is one of those
questions that people tend to have very strong feelings about.  Doing
something at the last minute risks people not feeling that they had an
adequate time to express those feelings before something got shipped.
Not everybody reads this list every day, or tests every new commit as
soon as it goes into the tree.


I agree with Andres and Robert.  This patch should be pushed to PG13.

I'll do that on March 8 unless there is a compelling argument not to.

Regards,
--
-David
da...@pgmasters.net

Re: \describe*

[Corey Huinker]

>
>
>> - Tab completion for \descibe-verbose.
>> I know that \d+ tab completion is also not there, but I think we must
>> have tab completion for \descibe-verbose.
>>
>> postgres=# \describe-
>> \describe-extension
>>  \describe-replication-publication \describe-user-mapping
>> \describe-foreign-data-wrapper
>> \describe-replication-subscription\describe-view
>> \describe-foreign-server  \describe-role
>>   \describe-window-function
>> \describe-foreign-table   \describe-rule
>>  ...
>>
>
I just confirmed that there isn't tab completion for the existing S/+
options, so it's hard to justify them for the equivalent verbose suffixes.



> (1 row)
>> Invalid command \describe. Try \? for help.
>>
>>
>> I think this status is causing the problem.
>>
>>
>>
>> +   /*
>> standard listing of interesting things */
>> +   success =
>> listTables("tvmsE", NULL, show_verbose, show_system);
>> +   }
>> +   status = PSQL_CMD_UNKNOWN;
>>
>>
I'll look into this, thanks!



> - Confusion about \desc and \desC
>> There is confusion while running the \desc command. I know the problem,
>> but the user may confuse by this.
>> postgres=# \desC
>>List of foreign servers
>>  Name | Owner | Foreign-data wrapper
>> --+---+--
>> (0 rows)
>>
>> postgres=# \desc
>> Invalid command \desc. Try \? for help.
>>
>> - Auto-completion of commands.
>> There is some more confusion in the completion of commands.
>>
>> This command shows List of aggregates.
>> postgres=# \describe-aggregate-function
>>  List of aggregate functions
>>  Schema | Name | Result data type | Argument data types | Description
>> +--+--+-+-
>> (0 rows)
>>
>>
>>
>> This command shows a list of relation "\d"
>> postgres=# \describe-aggregatE-function
>> List of relations
>>  Schema | Name | Type  |  Owner
>> +--+---+-
>>  public | foo  | table | vagrant
>> (1 row)
>>
>> This command also shows a list of relations "\d".
>> postgres=# \describe-aggr
>> List of relations
>>  Schema | Name | Type  |  Owner
>> +--+---+-
>>  public | foo  | table | vagrant
>> (1 row)
>>
>> This command shows error messages.
>> postgres=# \descr
>> Invalid command \descr. Try \? for help.
>>
>>
I will look into it.



>
>> I have done a brief code review except for the documentation code. I
>> don't like this code
>>
>> if (cmd_match(cmd,"describe-aggregate-function"))
>>
>>  success = describeAggregates(pattern, show_verbose, show_system);
>>  else if (cmd_match(cmd,
>> "describe-access-method"))
>>  success = describeAccessMethods(pattern,
>> show_verbose);
>>  else if (cmd_match(cmd,
>> "describe-tablespace"))
>>  success = describeTablespaces(pattern,
>> show_verbose);
>>  else if (cmd_match(cmd,
>> "describe-conversion"))
>>  success = listConversions(pattern,
>> show_verbose, show_system);
>>  else if (cmd_match(cmd, "describe-cast"))
>>  success = listCasts(pattern, show_verbose
>>
>>
>> This can be achieved with the list/array/hash table, so I have changed
>> that code in the attached patch just for a sample if you want I can do that
>> for whole code.
>>
>
There's some problems with a hash table. The function signatures vary quite
a lot, and some require additional psql_scan_slash_options to be called.
The hash option, if implemented, probably should be expanded to all slash
commands, at which point maybe it belongs in psqlscanslash.l...

>

Re: \describe*

[Ibrar Ahmed]

Hi Corey,

Here is the modified patch (sample).



On Mon, Mar 4, 2019 at 7:02 PM Ibrar Ahmed  wrote:

> The following review has been posted through the commitfest application:
> make installcheck-world:  not tested
> Implements feature:   not tested
> Spec compliant:   not tested
> Documentation:not tested
>
> Thanks for the patch, I have reviewed the patch and have some comments
> about the patch. The review includes the testing of the patch along with
> some code review.
>
> Here are my testings results,
>
> - Tab completion for \descibe-verbose.
> I know that \d+ tab completion is also not there, but I think we must have
> tab completion for \descibe-verbose.
>
> postgres=# \describe-
> \describe-extension
>  \describe-replication-publication \describe-user-mapping
> \describe-foreign-data-wrapper
> \describe-replication-subscription    \describe-view
> \describe-foreign-server  \describe-role
>   \describe-window-function
> \describe-foreign-table   \describe-rule
>  ...
>
>
> - Error message in each command.
> There is an error message after each command, here is the example.
> postgres=# \describe
> List of relations
>  Schema | Name | Type  |  Owner
> ----+--+---+-
>  public | foo  | table | vagrant
>
> (1 row)
> Invalid command \describe. Try \? for help.
>
>
> I think this status is causing the problem.
>
>
>
> +   /*
> standard listing of interesting things */
> +   success =
> listTables("tvmsE", NULL, show_verbose, show_system);
> +   }
> +   status = PSQL_CMD_UNKNOWN;
>
>
>
>
> - Confusion about \desc and \desC
> There is confusion while running the \desc command. I know the problem,
> but the user may confuse by this.
> postgres=# \desC
>List of foreign servers
>  Name | Owner | Foreign-data wrapper
> --+---+--
> (0 rows)
>
> postgres=# \desc
> Invalid command \desc. Try \? for help.
>
> - Auto-completion of commands.
> There is some more confusion in the completion of commands.
>
> This command shows List of aggregates.
> postgres=# \describe-aggregate-function
>  List of aggregate functions
>  Schema | Name | Result data type | Argument data types | Description
> +--+--+-+-
> (0 rows)
>
>
>
> This command shows a list of relation "\d"
> postgres=# \describe-aggregatE-function
> List of relations
>  Schema | Name | Type  |  Owner
> +--+---+-
>  public | foo  | table | vagrant
> (1 row)
>
> This command also shows a list of relations "\d".
> postgres=# \describe-aggr
> List of relations
>  Schema | Name | Type  |  Owner
> +--+---+-
>  public | foo  | table | vagrant
> (1 row)
>
> This command shows error messages.
> postgres=# \descr
> Invalid command \descr. Try \? for help.
>
> ...
>
>
> Code review.
> -
>
> I have done a brief code review except for the documentation code. I don't
> like this code
>
> if (cmd_match(cmd,"describe-aggregate-function"))
>
>  success = describeAggregates(pattern, show_verbose, show_system);
>  else if (cmd_match(cmd,
> "describe-access-method"))
>  success = describeAccessMethods(pattern,
> show_verbose);
>  else if (cmd_match(cmd,
> "describe-tablespace"))
>  success = describeTablespaces(pattern,
> show_verbose);
>  else if (cmd_match(cmd,
> "describe-conversion"))
>  success = listConversions(pattern,
> show_verbose, show_system);
>  else if (cmd_match(cmd, "describe-cast"))
>  success = listCasts(pattern, show_verbose
>
>
> This can be achieved with the list/array/hash table, so I have changed
> that code in the attached patch just for a sample if you want I can do that
> for whole code.
>
> --
> Ibrar Ahmed
>
> The new status of this patch is: Waiting on Author
>


-- 
Ibrar Ahmed


0001-Add-describe-commands-to-compliment-d-commands-ibrar-v2.patch
Description: Binary data