Thread: psql: Could we get "-- " prefixing on the **** QUERY **** outputs? (ECHO_HIDDEN)

This would be a trivial change.  Willing to do it, and push it.

In effect, we have this GREAT feature:
\set ECHO_HIDDON on

Which outputs a bunch of queries (as you all know).
But somehow nobody thought that a user might want to paste ALL of the queries into their query editor, or even into another psql session, via (\e)
and NOT get a ton of syntax errors?

As an example: (added -- and a space)

-- ********* QUERY **********
SELECT c2.relname, i.indisprimary, i.indisunique, i.indisclustered, i.indisvalid, pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),
  pg_catalog.pg_get_constraintdef(con.oid, true), contype, condeferrable, condeferred, i.indisreplident, c2.reltablespace
FROM pg_catalog.pg_class c, pg_catalog.pg_class c2, pg_catalog.pg_index i
  LEFT JOIN pg_catalog.pg_constraint con ON (conrelid = i.indrelid AND conindid = i.indexrelid AND contype IN ('p','u','x'))
WHERE c.oid = '21949943' AND c.oid = i.indrelid AND i.indexrelid = c2.oid
ORDER BY i.indisprimary DESC, c2.relname;
-- **************************

-- ********* QUERY **********
SELECT pol.polname, pol.polpermissive,
  CASE WHEN pol.polroles = '{0}' THEN NULL ELSE pg_catalog.array_to_string(array(select rolname from pg_catalog.pg_roles where oid = any (pol.polroles) order by 1),',') END,
  pg_catalog.pg_get_expr(pol.polqual, pol.polrelid),
  pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid),
  CASE pol.polcmd
    WHEN 'r' THEN 'SELECT'
    WHEN 'a' THEN 'INSERT'
    WHEN 'w' THEN 'UPDATE'
    WHEN 'd' THEN 'DELETE'
    END AS cmd
FROM pg_catalog.pg_policy pol
WHERE pol.polrelid = '21949943' ORDER BY 1;
-- **************************

Kirk...
Hi

Dne po 15. 5. 2023 8:01 uživatel Kirk Wolak <wolakk@gmail.com> napsal:
This would be a trivial change.  Willing to do it, and push it.

In effect, we have this GREAT feature:
\set ECHO_HIDDON on

Which outputs a bunch of queries (as you all know).
But somehow nobody thought that a user might want to paste ALL of the queries into their query editor, or even into another psql session, via (\e)
and NOT get a ton of syntax errors?

As an example: (added -- and a space)

-- ********* QUERY **********
SELECT c2.relname, i.indisprimary, i.indisunique, i.indisclustered, i.indisvalid, pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),
  pg_catalog.pg_get_constraintdef(con.oid, true), contype, condeferrable, condeferred, i.indisreplident, c2.reltablespace
FROM pg_catalog.pg_class c, pg_catalog.pg_class c2, pg_catalog.pg_index i
  LEFT JOIN pg_catalog.pg_constraint con ON (conrelid = i.indrelid AND conindid = i.indexrelid AND contype IN ('p','u','x'))
WHERE c.oid = '21949943' AND c.oid = i.indrelid AND i.indexrelid = c2.oid
ORDER BY i.indisprimary DESC, c2.relname;
-- **************************

-- ********* QUERY **********
SELECT pol.polname, pol.polpermissive,
  CASE WHEN pol.polroles = '{0}' THEN NULL ELSE pg_catalog.array_to_string(array(select rolname from pg_catalog.pg_roles where oid = any (pol.polroles) order by 1),',') END,
  pg_catalog.pg_get_expr(pol.polqual, pol.polrelid),
  pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid),
  CASE pol.polcmd
    WHEN 'r' THEN 'SELECT'
    WHEN 'a' THEN 'INSERT'
    WHEN 'w' THEN 'UPDATE'
    WHEN 'd' THEN 'DELETE'
    END AS cmd
FROM pg_catalog.pg_policy pol
WHERE pol.polrelid = '21949943' ORDER BY 1;
-- **************************

Kirk...

This looks little bit strange

What about /* comments

Like

/******* Query ********/

Or just 

-------- Query --------

Regards

Pavel
On Mon, May 15, 2023 at 2:37 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
Hi

Dne po 15. 5. 2023 8:01 uživatel Kirk Wolak <wolakk@gmail.com> napsal:
This would be a trivial change.  Willing to do it, and push it.

In effect, we have this GREAT feature:
\set ECHO_HIDDON on
-- **************************

Kirk...

This looks little bit strange

What about /* comments

Like

/******* Query ********/

Or just 

-------- Query --------

Regards

Pavel

Actually, I am open to suggestions.
/* */
Are good comments, usually safer! 
On Mon, 2023-05-15 at 08:37 +0200, Pavel Stehule wrote:
> Dne po 15. 5. 2023 8:01 uživatel Kirk Wolak <wolakk@gmail.com> napsal:
> > This would be a trivial change.  Willing to do it, and push it.
> >
> > In effect, we have this GREAT feature:
> > \set ECHO_HIDDON on
> >
> > Which outputs a bunch of queries (as you all know).
> > But somehow nobody thought that a user might want to paste ALL of the queries into their query editor, or even into
anotherpsql session, via (\e) 
> > and NOT get a ton of syntax errors?
> >
> > As an example: (added -- and a space)
> >
> > -- ********* QUERY **********
> > SELECT c2.relname, i.indisprimary, i.indisunique, i.indisclustered, i.indisvalid,
pg_catalog.pg_get_indexdef(i.indexrelid,0, true), 
> >   pg_catalog.pg_get_constraintdef(con.oid, true), contype, condeferrable, condeferred, i.indisreplident,
c2.reltablespace
> > FROM pg_catalog.pg_class c, pg_catalog.pg_class c2, pg_catalog.pg_index i
> >   LEFT JOIN pg_catalog.pg_constraint con ON (conrelid = i.indrelid AND conindid = i.indexrelid AND contype IN
('p','u','x'))
> > WHERE c.oid = '21949943' AND c.oid = i.indrelid AND i.indexrelid = c2.oid
> > ORDER BY i.indisprimary DESC, c2.relname;
> > -- **************************
>
> This looks little bit strange
>
> What about /* comments
>
> Like
>
> /******* Query ********/
>
> Or just 
>
> -------- Query --------

+1 for either of Pavel's suggestions.

Yours,
Laurenz Albe



Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Mon, 2023-05-15 at 08:37 +0200, Pavel Stehule wrote:
>> This looks little bit strange
>> 
>> What about /* comments
>> Like
>> /******* Query ********/
>> Or just
>> -------- Query --------

> +1 for either of Pavel's suggestions.

+1.  Probably the slash-star way would be less visually surprising
to people who are used to the current output.

Checking the psql source code for "****", I see that the single-step
feature could use the same treatment.

            regards, tom lane



On 2023-May-15, Tom Lane wrote:

> Laurenz Albe <laurenz.albe@cybertec.at> writes:
> > On Mon, 2023-05-15 at 08:37 +0200, Pavel Stehule wrote:
> >> This looks little bit strange
> >> 
> >> What about /* comments
> >> Like
> >> /******* Query ********/
> >> Or just
> >> -------- Query --------
> 
> > +1 for either of Pavel's suggestions.
> 
> +1.  Probably the slash-star way would be less visually surprising
> to people who are used to the current output.

It's worth considering what will readline history do with the comment.
As I recall, we keep /* comments */ together with the query that
follows, but the -- comments are keep in a separate history entry.
So that's one more reason to prefer /*  */

(To me, that also suggests to remove the asterisk line after each query,
and to keep just the one before.)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Sallah, I said NO camels! That's FIVE camels; can't you count?"
(Indiana Jones)



Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> It's worth considering what will readline history do with the comment.
> As I recall, we keep /* comments */ together with the query that
> follows, but the -- comments are keep in a separate history entry.
> So that's one more reason to prefer /*  */

Good point.

> (To me, that also suggests to remove the asterisk line after each query,
> and to keep just the one before.)

Meh ... the one after serves to separate a query from its output.

            regards, tom lane



On Mon, May 15, 2023 at 10:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> It's worth considering what will readline history do with the comment.
> As I recall, we keep /* comments */ together with the query that
> follows, but the -- comments are keep in a separate history entry.
> So that's one more reason to prefer /*  */

Good point.

> (To me, that also suggests to remove the asterisk line after each query,
> and to keep just the one before.)

Meh ... the one after serves to separate a query from its output.

                        regards, tom lane

Actually, I love the feedback!

I just tested whether or not you see the trailing comment line.  And I ONLY see it in the windows version of PSQL.
And ONLY if you paste it directly in at the command line.
[Because it sends the text line by line, I assume]

Further Testing:

calling with: psql -f  -- no output of the comments (or the query is seen)  -- Windows/Linux

with \e editing... In Linux nothing is displayed from the query!

with \e editing in Windows... I found it buggy when I tossed in (\pset pager 0) as the first line.  It blew everything up (LOL)
\pset: extra argument "attcollation," ignored
\pset: extra argument "a.attidentity," ignored
\pset: extra argument "a.attgenerated" ignored
\pset: extra argument "FROM" ignored
\pset: extra argument "pg_catalog.pg_attribute" ignored


With that said, I DEFINITELY Move to Remove the secondary comment.  It's just noise.
and /* */ comments it will be for the topside.

Also, I will take a quick peek at the parse failure that is in windows \e
[Which always does this weird doubling of lines].  But no promises here.  It will be good enough to identify the problem.

Kirk...

On Mon, May 15, 2023 at 9:05 PM Kirk Wolak <wolakk@gmail.com> wrote:
On Mon, May 15, 2023 at 10:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> It's worth considering what will readline history do with the comment.

Hmmm... We could put a SPACE before the comment, that usually stops readline from saving it?

Meh ... the one after serves to separate a query from its output.

                        regards, tom lane


I just tested whether or not you see the trailing comment line.  And I ONLY see it in the windows version of PSQL.
And ONLY if you paste it directly in at the command line.
[Because it sends the text line by line, I assume]

,,,With that said, I DEFINITELY Move to Remove the secondary comment.  It's just noise.
and /* */ comments it will be for the topside.


Here's the patch.  I removed touching on .po files.
I made the change apply to the logging (fair warning) for consistency.

All feedback is welcome.  These small patches help me work through the process.

Kirk...
OUTPUT:
/********* QUERY **********/
SELECT c.oid::pg_catalog.regclass
FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
WHERE c.oid = i.inhparent AND i.inhrelid = '24577'
  AND c.relkind != 'p' AND c.relkind != 'I'
ORDER BY inhseqno;

/********* QUERY **********/
SELECT c.oid::pg_catalog.regclass, c.relkind, inhdetachpending, pg_catalog.pg_get_expr(c.relpartbound, c.oid)
FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
WHERE c.oid = i.inhrelid AND i.inhparent = '24577'
ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;

                           Table "public.t1"
 Column |  Type  | Collation | Nullable |           Default            
--------+--------+-----------+----------+------------------------------
 id     | bigint |           | not null | generated always as identity
Indexes:
    "t1_pkey" PRIMARY KEY, btree (id)

Attachment
On Wed, 2023-05-17 at 13:39 -0400, Kirk Wolak wrote:
> Here's the patch.

You removed the ******** QUERY ******** at the end of the query.
I think we should keep that (as comments, of course).  People
are used to the current output, and it is nice to have a clear
visual marker at the end of what isn't normally part of "psql"
output.

"okbob" should be "Pavel Stehule".

Yours,
Laurenz Albe



Laurenz Albe <laurenz.albe@cybertec.at> writes:
> You removed the ******** QUERY ******** at the end of the query.
> I think we should keep that (as comments, of course).  People
> are used to the current output, and it is nice to have a clear
> visual marker at the end of what isn't normally part of "psql"
> output.

I agree.  These considerations of what shows up in the readline
log if you choose to copy-and-paste seem far secondary to the
readability of the terminal output in the first place.

Also, you'd have to avoid copying-and-pasting the query output
anyway, so I'm not entirely sold that there's much of
a usability gain here.

            regards, tom lane



On Wed, May 17, 2023 at 2:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Laurenz Albe <laurenz.albe@cybertec.at> writes:
> You removed the ******** QUERY ******** at the end of the query.

Fixed 
Also Fixed Pavel's name.
Also Added Laurenze as a Reviewed By: (not sure, never want to NOT ack someone)

Also, you'd have to avoid copying-and-pasting the query output
anyway, so I'm not entirely sold that there's much of
a usability gain here.
 
My output never contains query output results intermixed.  I get a handful of queries.
Then I get the output of the "\d t1"  (Which makes me wonder if I am doing something wrong,
or there is another use case I should be testing).

I labelled this v2.  I also edited the Thread: (I realized I can find the thread, go to the Whole Thread,
and then include the link to the first item in the thread.  I assume that is what's expected).

Kirk...

psql> 
create table t1(id bigint not null primary key generated always as identity);
\set ECHO_HIDDEN on
\d t1

Generates:
/********* QUERY **********/
... Clipped ...
FROM pg_catalog.pg_publication p
WHERE p.puballtables AND pg_catalog.pg_relation_is_publishable('24577')
ORDER BY 1;
/**************************/

/********* QUERY **********/
SELECT c.oid::pg_catalog.regclass
FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
WHERE c.oid = i.inhparent AND i.inhrelid = '24577'
  AND c.relkind != 'p' AND c.relkind != 'I'
ORDER BY inhseqno;
/**************************/

/********* QUERY **********/
SELECT c.oid::pg_catalog.regclass, c.relkind, inhdetachpending, pg_catalog.pg_get_expr(c.relpartbound, c.oid)
FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
WHERE c.oid = i.inhrelid AND i.inhparent = '24577'
ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
/**************************/

                           Table "public.t1"
 ... End Clip...
-- NOTICE: there is no output between queries using ECHO_HIDDEN
Attachment

Re: psql: Could we get "-- " prefixing on the **** QUERY **** outputs? (ECHO_HIDDEN)

From
"Andrey M. Borodin"
Date:

> On 18 May 2023, at 02:23, Kirk Wolak <wolakk@gmail.com> wrote:
>
> I labelled this v2.

+1 to the feature and the patch looks good to me.

I have a question, but mostly for my own knowledge. Translation changes seem trivial for all languages, do we typically
fix.po files in such cases? Or do we leave it to translators to revise the stuff? 

Thanks!

Best regards, Andrey Borodin.




On 2023-May-19, Andrey M. Borodin wrote:

> I have a question, but mostly for my own knowledge. Translation
> changes seem trivial for all languages, do we typically fix .po files
> in such cases? Or do we leave it to translators to revise the stuff?

The translations use a completely separate source repository, so even if
somebody were to patch them in postgresql.git, their changes would be
overwritten next time they are copied from the translation repo anyway.
Just leave it to the translators.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"In Europe they call me Niklaus Wirth; in the US they call me Nickel's worth.
 That's because in Europe they call me by name, and in the US by value!"



I took a look at this patch and changed a couple things:

 * I made a similar adjustment to a few lines that seem to have been
   missed.
 * I removed a couple of asterisks from the adjusted lines in order to
   maintain the existing line lengths.

Barring additional feedback, I think this is ready for commit.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment


st 26. 7. 2023 v 6:22 odesílatel Nathan Bossart <nathandbossart@gmail.com> napsal:
I took a look at this patch and changed a couple things:

 * I made a similar adjustment to a few lines that seem to have been
   missed.
 * I removed a couple of asterisks from the adjusted lines in order to
   maintain the existing line lengths.

Barring additional feedback, I think this is ready for commit.


+1

Pavel

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Wed, Jul 26, 2023 at 08:06:37AM +0200, Pavel Stehule wrote:
> st 26. 7. 2023 v 6:22 odesílatel Nathan Bossart <nathandbossart@gmail.com>
> napsal:
>> Barring additional feedback, I think this is ready for commit.
>>
>>
> +1

Great.  I spent some time on the commit message in v4.  I plan to commit
this shortly.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment
On Wed, Jul 26, 2023 at 02:39:25PM -0700, Nathan Bossart wrote:
> Great.  I spent some time on the commit message in v4.  I plan to commit
> this shortly.

Committed.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com



On Wed, Jul 26, 2023 at 5:39 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
On Wed, Jul 26, 2023 at 08:06:37AM +0200, Pavel Stehule wrote:
> st 26. 7. 2023 v 6:22 odesílatel Nathan Bossart <nathandbossart@gmail.com>
> napsal:
>> Barring additional feedback, I think this is ready for commit.
>>
>>
> +1

Great.  I spent some time on the commit message in v4.  I plan to commit
this shortly.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Curious about this.  I expected to see the comments?  (is there a chance that the translation piece is kicking in reverting them)?
(expecting / ********* QUERY **********/)

01:05:47 devuser@nctest= > \echo :VERSION_NAME  :VERSION_NUM
16.0 (Ubuntu 16.0-1.pgdg22.04+1) 160000
01:05:57 devuser@nctest= > \dn public
********* QUERY **********
SELECT n.nspname AS "Name",
  pg_catalog.pg_get_userbyid(n.nspowner) AS "Owner"
FROM pg_catalog.pg_namespace n
WHERE n.nspname OPERATOR(pg_catalog.~) '^(public)$' COLLATE pg_catalog.default
ORDER BY 1;
**************************

********* QUERY **********
SELECT pubname
FROM pg_catalog.pg_publication p
     JOIN pg_catalog.pg_publication_namespace pn ON p.oid = pn.pnpubid
     JOIN pg_catalog.pg_namespace n ON n.oid = pn.pnnspid
WHERE n.nspname = 'public'
ORDER BY 1
**************************

      List of schemas
  Name  |       Owner
--------+-------------------
 public | pg_database_owner
(1 row)


 
Hi

út 24. 10. 2023 v 7:10 odesílatel Kirk Wolak <wolakk@gmail.com> napsal:
On Wed, Jul 26, 2023 at 5:39 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
On Wed, Jul 26, 2023 at 08:06:37AM +0200, Pavel Stehule wrote:
> st 26. 7. 2023 v 6:22 odesílatel Nathan Bossart <nathandbossart@gmail.com>
> napsal:
>> Barring additional feedback, I think this is ready for commit.
>>
>>
> +1

Great.  I spent some time on the commit message in v4.  I plan to commit
this shortly.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Curious about this.  I expected to see the comments?  (is there a chance that the translation piece is kicking in reverting them)?
(expecting / ********* QUERY **********/)

01:05:47 devuser@nctest= > \echo :VERSION_NAME  :VERSION_NUM
16.0 (Ubuntu 16.0-1.pgdg22.04+1) 160000
01:05:57 devuser@nctest= > \dn public
********* QUERY **********
SELECT n.nspname AS "Name",
  pg_catalog.pg_get_userbyid(n.nspowner) AS "Owner"
FROM pg_catalog.pg_namespace n
WHERE n.nspname OPERATOR(pg_catalog.~) '^(public)$' COLLATE pg_catalog.default
ORDER BY 1;
**************************

********* QUERY **********
SELECT pubname
FROM pg_catalog.pg_publication p
     JOIN pg_catalog.pg_publication_namespace pn ON p.oid = pn.pnpubid
     JOIN pg_catalog.pg_namespace n ON n.oid = pn.pnnspid
WHERE n.nspname = 'public'
ORDER BY 1
**************************

      List of schemas
  Name  |       Owner
--------+-------------------
 public | pg_database_owner
(1 row)


It is working in psql 17, not in psql 16

(2023-10-24 07:14:35) postgres=# \echo :VERSION_NAME  :VERSION_NUM
17devel 170000
(2023-10-24 07:14:37) postgres=# \l+
/******** QUERY *********/
SELECT
  d.datname as "Name",
  pg_catalog.pg_get_userbyid(d.datdba) as "Owner",
  pg_catalog.pg_encoding_to_char(d.encoding) as "Encoding",
  CASE d.datlocprovider WHEN 'c' THEN 'libc' WHEN 'i' THEN 'icu' END AS "Locale Provider",
...