proposal psql \gdesc

2017-04-28 6:08 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hi

Sometimes I have to solve the result types of some query. It is invisible
in psql. You have to materialize table or you have to create view. Now,
when we can enhance \g command, we can introduce query describing

some like

select a, b from foo
\gdesc

| type | length | collation | ....
------------------------------------------------
a | varchar | 30 |
b | numeric | 20 |

here is the patch. It is based on PQdescribePrepared result.

postgres=# select * from pg_proc \gdesc
┌─────────────────┬──────────────┐
│ Name │ Type │
╞═════════════════╪══════════════╡
│ proname │ name │
│ pronamespace │ oid │
│ proowner │ oid │
│ prolang │ oid │
│ procost │ real │
│ prorows │ real │
│ provariadic │ oid │
│ protransform │ regproc │
│ proisagg │ boolean │
│ proiswindow │ boolean │
│ prosecdef │ boolean │
│ proleakproof │ boolean │
│ proisstrict │ boolean │
│ proretset │ boolean │
│ provolatile │ "char" │
│ proparallel │ "char" │
│ pronargs │ smallint │
│ pronargdefaults │ smallint │
│ prorettype │ oid │
│ proargtypes │ oidvector │
│ proallargtypes │ oid[] │
│ proargmodes │ "char"[] │
│ proargnames │ text[] │
│ proargdefaults │ pg_node_tree │
│ protrftypes │ oid[] │
│ prosrc │ text │
│ probin │ text │
│ proconfig │ text[] │
│ proacl │ aclitem[] │
└─────────────────┴──────────────┘
(29 rows)

Show quoted text

What do you think about this idea?

Regards

Pavel

Hello Pavel,

Sometimes I have to solve the result types of some query. It is invisible
in psql.

Indeed. My 0.02€ about this patch:

About the feature:

It looks useful to allow to show the resulting types of a query.

About the code:

Patch applies cleanly and compiles.

I'm afraid that re-using the "results" variable multiple times results in
memory leaks... ISTM that new variables should be used when going down the
nested conditions, and all cleanup should be done where and when
necessary.

Also, maybe it would be better if the statement is cleaned up server side
at the end of the execution. Not sure how to achieve that, though, libpq
seems to lack the relevant function:-(

"""although there is no libpq function for deleting a prepared
statement, the SQL DEALLOCATE statement can be used for that purpose."""

Hmmm... I have not found how to use DEALLOCATE to cleanup an unnamed
statement, it does not allow a "zero-length" name. Maybe it could be
extended somehow, or a function could be provided for the purpose, eg
by passing a NULL query to PQprepare...

Resetting "gdesc flag" could be done only when the flag was true, at the
end of the if, rather than on each query.

I understand that the second level query is to retrieve the type names in
place of the Oid returned by QPftype?

The pg_malloc length computation looks a little bit arbitrary. Would it
make sense to use PQescapeLiteral instead?

About the documentation:

I would suggest some rewording, maybe:

"Show the description of the result of the current query buffer without
actually executing it, by considering it a prepared statement."

About tests:

There should be some non-regression tests added, maybe something like:

SELECT
NULL AS zero,
1 AS one,
2.0 AS two,
'three' AS three,
$1 AS four,
CURRENT_DATE AS now
-- ...
\gdesc

And also case which trigger an error, eg:

SELECT $1 AS unknown_type \gdesc
SELECT 1 + \gdesc

Some fun:

PREPARE test AS SELECT 1;
EXECUTE test \gdesc
...

I'm unsure about some error messages, but it may be unrelated to this
patch:

calvin=# SELECT '1 km'::unit AS foo, $2 as boo \gdesc
ERROR: could not determine data type of parameter $1

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello Pavel,

A complement to my previous comments:

Also, maybe it would be better if the statement is cleaned up server side at
the end of the execution. Not sure how to achieve that, though, libpq seems
to lack the relevant function:-(

"""although there is no libpq function for deleting a prepared
statement, the SQL DEALLOCATE statement can be used for that purpose."""

Hmmm... I have not found how to use DEALLOCATE to cleanup an unnamed
statement, it does not allow a "zero-length" name. Maybe it could be extended
somehow, or a function could be provided for the purpose, eg
by passing a NULL query to PQprepare...

After giving it some thoughts, I see three possible solutions:

0. Do nothing about it.
I would prefer the prepare is cleaned up.

1. assign a special name, eg "_psql_gdesc_", so that
DEALLOCATE "_psql_gdesc_" can be issued afterwards.

2. allow executing DEALLOCATE "";

3. add the missing PQdeallocate function to libpq?

Version 2 is server side, so it would not be compatible when connected
to server running previous versions. Not desirable.

Version 3 may have implication at the protocol level and server side, if
so it does not seem desirable to introduce such a change.

So maybe only version 1 is possible.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi

2017-05-08 9:08 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

A complement to my previous comments:

Also, maybe it would be better if the statement is cleaned up server side

at the end of the execution. Not sure how to achieve that, though, libpq
seems to lack the relevant function:-(

"""although there is no libpq function for deleting a prepared
statement, the SQL DEALLOCATE statement can be used for that purpose."""

Hmmm... I have not found how to use DEALLOCATE to cleanup an unnamed
statement, it does not allow a "zero-length" name. Maybe it could be
extended somehow, or a function could be provided for the purpose, eg
by passing a NULL query to PQprepare...

After giving it some thoughts, I see three possible solutions:

0. Do nothing about it.
I would prefer the prepare is cleaned up.

1. assign a special name, eg "_psql_gdesc_", so that
DEALLOCATE "_psql_gdesc_" can be issued afterwards.

2. allow executing DEALLOCATE "";

3. add the missing PQdeallocate function to libpq?

Version 2 is server side, so it would not be compatible when connected to
server running previous versions. Not desirable.

Version 3 may have implication at the protocol level and server side, if
so it does not seem desirable to introduce such a change.

So maybe only version 1 is possible.

The doc says about unnamed prepared statements - any new unnamed prepared
statement deallocates previous by self. From psql environment is not
possible to create unnamed prepared statement. So there are not any
possible conflict and only one unnamed prepared statement can exists. The
overhead is like any call of PLpgSQL function where any embedded SQLs are
prepared implicitly. So @0 is from my perspective safe. Looks so unnamed PP
was designed for this short life PP.

I prefer @0 agaisnt @1 because workflow is simple and robust. If unnamed PP
doesn't exists, then it will be created, else it will be replaced. @1 has
little bit more complex workflow, because there is not command like
DEALLOCATE IF EXISTS, so I have to ensure deallocation in all possible
ways. Another reason for @0 is not necessity to generate some auxiliary
name.

So in this case, I thinking @0 is good enough way (due unnamed PP behave),
and can be enhanced by @3, but @3 requires wide discussion about design
(and can be overkill for \gdesc command) and should be problem everywhere
you use new client against old server. Same problem (you mentioned) has @2.

My opinion in this case is not too strong - just I see the advantages of @0
(robust and simple) nice. The question is about cost of unwanted allocated
PP to end of session.

Regards

Pavel

Show quoted text

--
Fabien.

Hello Pavel,

After giving it some thoughts, I see three possible solutions:

0. Do nothing about it.
I would prefer the prepare is cleaned up.

1. assign a special name, eg "_psql_gdesc_", so that
DEALLOCATE "_psql_gdesc_" can be issued afterwards.

[...]

The doc says about unnamed prepared statements - any new unnamed prepared
statement deallocates previous by self. From psql environment is not
possible to create unnamed prepared statement.

That is a good point. It seems that it is not possible to execute it
either.

I prefer @0 agaisnt @1 because workflow is simple and robust. If unnamed PP
doesn't exists, then it will be created, else it will be replaced. @1 has
little bit more complex workflow, because there is not command like
DEALLOCATE IF EXISTS, so I have to ensure deallocation in all possible
ways.

ISTM That it is only of the PQprepare succeeded, so there should be only
one point, at the end of the corresponding OK condition?

Another reason for @0 is not necessity to generate some auxiliary
name.

Yes. I do not like special names either. But I do not like keeping objects
lives if they are dead. Not sure which is worst.

My opinion in this case is not too strong - just I see the advantages of @0
(robust and simple) nice. The question is about cost of unwanted allocated
PP to end of session.

My opinion is not strong either, it is more the principle that I do not
like to let things forever live in the server while they are known dead.

Hmmm. Strange. For some obscure reason, the unnamed prepared statement
does not show in pg_prepared_statements:

calvin=# PREPARE test AS SELECT 2;
calvin=# EXECUTE test;
-- 2
calvin=# SELECT 1 AS one \gdesc
-- one | integer
calvin=# SELECT * FROM pg_prepared_statements ;
-- just one row:
-- test | PREPARE test AS SELECT 2; │7..

Conclusion: Maybe let it as solution 0 for the time being, with a comment
telling that it will be cleaned on the next unnamed PQprepare, and the
committer will have its opinion.

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2017-05-08 12:59 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

After giving it some thoughts, I see three possible solutions:

0. Do nothing about it.
I would prefer the prepare is cleaned up.

1. assign a special name, eg "_psql_gdesc_", so that
DEALLOCATE "_psql_gdesc_" can be issued afterwards.

[...]

The doc says about unnamed prepared statements - any new unnamed prepared
statement deallocates previous by self. From psql environment is not
possible to create unnamed prepared statement.

That is a good point. It seems that it is not possible to execute it
either.

I prefer @0 agaisnt @1 because workflow is simple and robust. If unnamed PP

doesn't exists, then it will be created, else it will be replaced. @1 has
little bit more complex workflow, because there is not command like
DEALLOCATE IF EXISTS, so I have to ensure deallocation in all possible
ways.

ISTM That it is only of the PQprepare succeeded, so there should be only
one point, at the end of the corresponding OK condition?

Another reason for @0 is not necessity to generate some auxiliary

name.

Yes. I do not like special names either. But I do not like keeping objects
lives if they are dead. Not sure which is worst.

My opinion in this case is not too strong - just I see the advantages of @0

(robust and simple) nice. The question is about cost of unwanted allocated
PP to end of session.

My opinion is not strong either, it is more the principle that I do not
like to let things forever live in the server while they are known dead.

Hmmm. Strange. For some obscure reason, the unnamed prepared statement
does not show in pg_prepared_statements:

looks like the design. Unnamed PP is near to PP created by PLpgSQL.

calvin=# PREPARE test AS SELECT 2;
calvin=# EXECUTE test;
-- 2
calvin=# SELECT 1 AS one \gdesc
-- one | integer
calvin=# SELECT * FROM pg_prepared_statements ;
-- just one row:
-- test | PREPARE test AS SELECT 2; │7..

Conclusion: Maybe let it as solution 0 for the time being, with a comment
telling that it will be cleaned on the next unnamed PQprepare, and the
committer will have its opinion.

good decision.

Thank you for review. I'll send new version early.

Regards

Pavel

Show quoted text

--
Fabien.

Hi

2017-05-07 22:55 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

Sometimes I have to solve the result types of some query. It is invisible

in psql.

Indeed. My 0.02€ about this patch:

About the feature:

It looks useful to allow to show the resulting types of a query.

About the code:

Patch applies cleanly and compiles.

I'm afraid that re-using the "results" variable multiple times results in
memory leaks... ISTM that new variables should be used when going down the
nested conditions, and all cleanup should be done where and when necessary.

Also, maybe it would be better if the statement is cleaned up server side
at the end of the execution. Not sure how to achieve that, though, libpq
seems to lack the relevant function:-(

"""although there is no libpq function for deleting a prepared
statement, the SQL DEALLOCATE statement can be used for that purpose."""

Hmmm... I have not found how to use DEALLOCATE to cleanup an unnamed
statement, it does not allow a "zero-length" name. Maybe it could be
extended somehow, or a function could be provided for the purpose, eg
by passing a NULL query to PQprepare...

Resetting "gdesc flag" could be done only when the flag was true, at the
end of the if, rather than on each query.

I don't think - now it is processed in sendquery_cleanup and it is
consistent with other flags. "if" is used only when some memory releasing
is necessary.

I understand that the second level query is to retrieve the type names in
place of the Oid returned by QPftype?

yes, DESCRIBE doesn't return text type names.

The pg_malloc length computation looks a little bit arbitrary. Would it
make sense to use PQescapeLiteral instead?

done

About the documentation:

I would suggest some rewording, maybe:

"Show the description of the result of the current query buffer without
actually executing it, by considering it a prepared statement."

done

About tests:

There should be some non-regression tests added, maybe something like:

SELECT
NULL AS zero,
1 AS one,
2.0 AS two,
'three' AS three,
$1 AS four,
CURRENT_DATE AS now
-- ...
\gdesc

And also case which trigger an error, eg:

done

SELECT $1 AS unknown_type \gdesc

It is not unknown type - the default placeholder type is text

SELECT 1 + \gdesc

Some fun:

PREPARE test AS SELECT 1;
EXECUTE test \gdesc
...

I'm unsure about some error messages, but it may be unrelated to this
patch:

calvin=# SELECT '1 km'::unit AS foo, $2 as boo \gdesc
ERROR: could not determine data type of parameter $1

Looks like messy PostgreSQL error message. You miss "$1" placeholder

attached updated patch

Regards

Pavel

Show quoted text

--
Fabien.

Hello Pavel,

Patch applies cleanly and compiles.

Idem for v2. "make check" ok. Tests look good.

I would suggest some rewording, maybe:

"Show the description of the result of the current query buffer without
actually executing it, by considering it a prepared statement."

done

Ok. If some native English speaker can clarify the sentence further, or
imprive it anyway, thanks in advance!

SELECT $1 AS unknown_type \gdesc

It is not unknown type - the default placeholder type is text

Indeed. I really meant something like:

calvin=# SELECT $1 + $2 \gdesc
ERROR: operator is not unique: unknown + unknown
...

More comments:

I propose that the help message could be "describe result of query without
executing it".

I found an issue. \gdesk fails when the command does not return a result:

calvin=# TRUNCATE pgbench_history \gdesc
ERROR: syntax error at or near ")"
LINE 2: (VALUES ) s (name, tp, tpm)

I guess the issue is that PQdescribePrepared returns an empty description,
which is fine, but then the second query should be skipped, and some
message should be output instead, like "no result" or whatever...

This need fixing, and a corresponding test should be added.

Also I would suggest to add a \g after the first test, which would execute
the current buffer after its description, to show that the current buffer
does indeed hold the query:

calvin=# SELECT 1 as one, ... \gdesc \g
-- one | int
-- ...
-- 1 | ...

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2017-05-09 18:15 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

Patch applies cleanly and compiles.

Idem for v2. "make check" ok. Tests look good.

I would suggest some rewording, maybe:

"Show the description of the result of the current query buffer without
actually executing it, by considering it a prepared statement."

done

Ok. If some native English speaker can clarify the sentence further, or
imprive it anyway, thanks in advance!

SELECT $1 AS unknown_type \gdesc

It is not unknown type - the default placeholder type is text

Indeed. I really meant something like:

calvin=# SELECT $1 + $2 \gdesc
ERROR: operator is not unique: unknown + unknown
...

More comments:

I propose that the help message could be "describe result of query without
executing it".

done

I found an issue. \gdesk fails when the command does not return a result:

calvin=# TRUNCATE pgbench_history \gdesc
ERROR: syntax error at or near ")"
LINE 2: (VALUES ) s (name, tp, tpm)

I guess the issue is that PQdescribePrepared returns an empty description,
which is fine, but then the second query should be skipped, and some
message should be output instead, like "no result" or whatever...

This need fixing, and a corresponding test should be added.

it is little bit worse. I cannot to distinguish between SELECT\gdesc and
TRUNCATE xxx\gdesc . All are valid commands and produce empty result, so
result of \gdesc command should be empty result too.

postgres=# truncate table xx\gdesc
┌──────┬──────┐
│ Name │ Type │
╞══════╪══════╡
└──────┴──────┘
(0 rows)

postgres=# select \gdesc
┌──────┬──────┐
│ Name │ Type │
╞══════╪══════╡
└──────┴──────┘
(0 rows)

Also I would suggest to add a \g after the first test, which would execute
the current buffer after its description, to show that the current buffer
does indeed hold the query:

calvin=# SELECT 1 as one, ... \gdesc \g
-- one | int
-- ...
-- 1 | ...

done

Regards

Pavel

Show quoted text

--
Fabien.

Hello Pavel,

[...] it is little bit worse. I cannot to distinguish between
SELECT\gdesc and TRUNCATE xxx\gdesc . All are valid commands and produce
empty result, so result of \gdesc command should be empty result too.

postgres=# truncate table xx\gdesc
О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫
О©╫ Name О©╫ Type О©╫
О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫
О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫
(0 rows)

Hmmm. At least it is better than the previous error.

What about detecting the empty result (eg PQntuples()==0?) and writing
"Empty result" instead of the strange looking empty table above? That
would just mean skipping the PrintQueryResult call in this case?

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2017-05-09 20:37 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

[...] it is little bit worse. I cannot to distinguish between SELECT\gdesc

and TRUNCATE xxx\gdesc . All are valid commands and produce empty result,
so result of \gdesc command should be empty result too.

postgres=# truncate table xx\gdesc
┌──────┬──────┐
│ Name │ Type │
╞══════╪══════╡
└──────┴──────┘
(0 rows)

Hmmm. At least it is better than the previous error.

What about detecting the empty result (eg PQntuples()==0?) and writing
"Empty result" instead of the strange looking empty table above? That would
just mean skipping the PrintQueryResult call in this case?

PQntuples == 0 every time - the query is not executed.

I am not sure, what is more correct. The "Empty result" string is not used
every time in psql. For the case "SELECT;" the empty table is correct. For
TRUNCATE and similar command I am not sure. The empty table is maybe
unusual, but it is valid - like "SELECT;". The implementation is not
problem in any case. The question is what is more natural for users a) the
string "Empty result", b) empty table.

I prefer @b due consistency with current behave (that is only reason, I
have not any other).

postgres=# select 'ahoj' where false;
┌──────────┐
│ ?column? │
╞══════════╡
└──────────┘
(0 rows)

Show quoted text

--
Fabien.

On 5/3/17 02:56, Pavel Stehule wrote:

Sometimes I have to solve the result types of some query. It is
invisible in psql. You have to materialize table or you have to
create view. Now, when we can enhance \g command, we can introduce
query describing

some like

select a, b from foo
\gdesc

| type | length | collation | ....
------------------------------------------------
a | varchar | 30 |
b | numeric | 20 |

here is the patch. It is based on PQdescribePrepared result.

I have often wished for functionality like this, so I'm in favor of
investigating this.

I don't think you need a separate call to prepare the query. You can
get the result column types using PQftype(). (Hmm, you can get the
typmod that way, but not the collation.)

My thinking in the past has been to put the column types either in the
column headers, like "colname (coltype)", or in the footer, along with
the actual query result.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2017-05-09 21:23 GMT+02:00 Peter Eisentraut <
peter.eisentraut@2ndquadrant.com>:

On 5/3/17 02:56, Pavel Stehule wrote:

Sometimes I have to solve the result types of some query. It is
invisible in psql. You have to materialize table or you have to
create view. Now, when we can enhance \g command, we can introduce
query describing

some like

select a, b from foo
\gdesc

| type | length | collation | ....
------------------------------------------------
a | varchar | 30 |
b | numeric | 20 |

here is the patch. It is based on PQdescribePrepared result.

I have often wished for functionality like this, so I'm in favor of
investigating this.

I don't think you need a separate call to prepare the query. You can
get the result column types using PQftype(). (Hmm, you can get the
typmod that way, but not the collation.)

the describe command is used and collation info is not available

looks to the attached patches

My thinking in the past has been to put the column types either in the
column headers, like "colname (coltype)", or in the footer, along with
the actual query result.

My first idea was like classic gui implementation

colname1
type
==========
data

but the header is not multi line.

Merging with result is another way, but mostly you don't need this info. So
special command looks better.

Show quoted text

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

What about detecting the empty result (eg PQntuples()==0?) and writing
"Empty result" instead of the strange looking empty table above? That would
just mean skipping the PrintQueryResult call in this case?

PQntuples == 0 every time - the query is not executed.

I meant to test the query which collects type names, which is executed?

Or check that PQnfields() == 0 on the PQdescribePrepared() result, so
that there is no need to execute the type name collection query?

For the case "SELECT;" the empty table is correct.

Ok. Then write "Empty table"?

For TRUNCATE and similar command I am not sure. The empty table is maybe
unusual, but it is valid - like "SELECT;".

I would partly disagree:

"SELECT;" does indeed return an empty relation, so I agree that an empty
table is valid whether spelled out as "Empty table" or explicitly.

However, ISTM that "TRUNCATE stuff;" does *NOT* return a relation, so
maybe "No table" would be ok, but not an empty table... ?!

So I could be okay with both:

SELECT \gdesc
-- "Empty table" or some other string
Or
-- Name | Type

Although I prefer the first one, because the second looks like a bug
somehow: I asked for a description, but nothing is described... even if
the answer is somehow valid, it looks pretty strange.

The same results do not realy suit "TRUNCATE Foo \gdesc", where "No table"
would seem more appropriate?

In both case, "Empty result" is kind of neutral, it does not promise a
table or not. Hmmm. At least not too much. Or maybe some other string such
as "Nothing" or "No result"?

Now I wonder whether the No vs Empty cases can be distinguished?

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2017-05-09 23:00 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

What about detecting the empty result (eg PQntuples()==0?) and writing

"Empty result" instead of the strange looking empty table above? That
would
just mean skipping the PrintQueryResult call in this case?

PQntuples == 0 every time - the query is not executed.

I meant to test the query which collects type names, which is executed?

How it can help?

Or check that PQnfields() == 0 on the PQdescribePrepared() result, so that
there is no need to execute the type name collection query?

For the case "SELECT;" the empty table is correct.

Ok. Then write "Empty table"?

For TRUNCATE and similar command I am not sure. The empty table is maybe

unusual, but it is valid - like "SELECT;".

I would partly disagree:

"SELECT;" does indeed return an empty relation, so I agree that an empty
table is valid whether spelled out as "Empty table" or explicitly.

However, ISTM that "TRUNCATE stuff;" does *NOT* return a relation, so
maybe "No table" would be ok, but not an empty table... ?!

So I could be okay with both:

SELECT \gdesc
-- "Empty table" or some other string
Or
-- Name | Type

Although I prefer the first one, because the second looks like a bug
somehow: I asked for a description, but nothing is described... even if the
answer is somehow valid, it looks pretty strange.

The same results do not realy suit "TRUNCATE Foo \gdesc", where "No table"
would seem more appropriate?

In both case, "Empty result" is kind of neutral, it does not promise a
table or not. Hmmm. At least not too much. Or maybe some other string such
as "Nothing" or "No result"?

Now I wonder whether the No vs Empty cases can be distinguished?

No with standard libpq API :(

I am sending a variant with message instead empty result.

Personally I prefer empty result instead message. It is hard to choose some
good text of this message. Empty result is just empty result for all cases.

Regards

Pavel

Show quoted text

--
Fabien.

Hello Pavel,

I am sending a variant with message instead empty result.

Thanks. Patch looks good, applies, make check ok, code is neat.

Personally I prefer empty result instead message.

Hmm. I think that this version is less likely to raise questions from
users, especially compared to having a somehow correct but strangely
looking description.

It is hard to choose some good text of this message. Empty result is
just empty result for all cases.

I'd suggest a very minor change: "No columns or command has no result"
(not -> no). If some English native speaker has a better suggestion,
fine with me.

Another good point of this version is that the type name query is
simplified because it does not need to handle an empty result, thus the
code is easier to understand.

A few other suggestions:

- could you update the comment on the type name query?
Maybe the comment can be simply removed?

- I'm wondering whether the Name & Type columns names should be
translatable. What do you think?

- Maybe tests could also exercise unnamed columns, eg:
SELECT 1, 2, 3 \gdesc \g

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi

2017-05-20 9:15 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

I am sending a variant with message instead empty result.

Thanks. Patch looks good, applies, make check ok, code is neat.

Personally I prefer empty result instead message.

Hmm. I think that this version is less likely to raise questions from
users, especially compared to having a somehow correct but strangely
looking description.

It is hard to choose some good text of this message. Empty result is just

empty result for all cases.

we will see

I'd suggest a very minor change: "No columns or command has no result"
(not -> no). If some English native speaker has a better suggestion, fine
with me.

changed

Another good point of this version is that the type name query is
simplified because it does not need to handle an empty result, thus the
code is easier to understand.

A few other suggestions:

- could you update the comment on the type name query?
Maybe the comment can be simply removed?

removed

- I'm wondering whether the Name & Type columns names should be
translatable. What do you think?

good idea - done

- Maybe tests could also exercise unnamed columns, eg:
SELECT 1, 2, 3 \gdesc \g

done

Regards

Pavel

Show quoted text

--
Fabien.

Hello Pavel,

- Maybe tests could also exercise unnamed columns, eg:
SELECT 1, 2, 3 \gdesc \g

done

Can't see it. No big deal, but if you put it it did not get through, and
there is a warning with git apply on the very last line of the patch which
may be linked to that:

psql-gdesc-05.patch:328: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

... especially as the two last tests are nearly the same now. I'm fine
with a "one line" test, could be with some unnamed columns so that it is
more different?

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2017-05-20 22:26 GMT+02:00 Fabien COELHO <coelho@cri.ensmp.fr>:

Hello Pavel,

- Maybe tests could also exercise unnamed columns, eg:

SELECT 1, 2, 3 \gdesc \g

done

Can't see it. No big deal, but if you put it it did not get through, and
there is a warning with git apply on the very last line of the patch which
may be linked to that:

psql-gdesc-05.patch:328: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

looks like pg_regress issue - more result files has extra blank line on
end. I am able to clean it only with \r on end of sql script - not sure
what is more worst - unrelated \command or this warning

... especially as the two last tests are nearly the same now. I'm fine
with a "one line" test, could be with some unnamed columns so that it is
more different?

ok - look on new version, please

Show quoted text

--
Fabien.