psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

Started by Andrew Dunstanabout 11 years ago10 messages

andrew@dunslane.net

about 11 years ago

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.
This looks on a somewhat cursory examination to be the only significant
place that doesn't. Is there any reason for this, or should I just
adjust it so that it uses PSQLexec like pretty much every other slash
command?

cheers

andrew

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

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Andrew Dunstan (#1)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

Andrew Dunstan <andrew@dunslane.net> writes:

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.
This looks on a somewhat cursory examination to be the only significant
place that doesn't. Is there any reason for this, or should I just
adjust it so that it uses PSQLexec like pretty much every other slash
command?

I think that was just an oversight in that patch. Adjust away.

regards, tom lane

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

Pavel Stehule

pavel.stehule@gmail.com

about 11 years ago

In reply to: Tom Lane (#2)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

2014-11-21 16:46 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:

Andrew Dunstan <andrew@dunslane.net> writes:

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.
This looks on a somewhat cursory examination to be the only significant
place that doesn't. Is there any reason for this, or should I just
adjust it so that it uses PSQLexec like pretty much every other slash
command?

I think that was just an oversight in that patch. Adjust away.

yes, it is open ticket

/messages/by-id/CAFj8pRCu3wV0MK4WYFBQDs1vosNmh5tXS2Gb=+fbojnW-1wzQg@mail.gmail.com

Regards

Pavel

Show quoted text

regards, tom lane

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

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Pavel Stehule (#3)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

On 11/21/2014 11:11 AM, Pavel Stehule wrote:

2014-11-21 16:46 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us
<mailto:tgl@sss.pgh.pa.us>>:

Andrew Dunstan <andrew@dunslane.net <mailto:andrew@dunslane.net>>
writes:

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.
This looks on a somewhat cursory examination to be the only

significant

place that doesn't. Is there any reason for this, or should I just
adjust it so that it uses PSQLexec like pretty much every other

slash

command?

I think that was just an oversight in that patch. Adjust away.

yes, it is open ticket

/messages/by-id/CAFj8pRCu3wV0MK4WYFBQDs1vosNmh5tXS2Gb=+fbojnW-1wzQg@mail.gmail.com

OK. it was so trivial I just did it.

cheers

andrew

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

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Andrew Dunstan (#4)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

Andrew Dunstan <andrew@dunslane.net> writes:

On 11/21/2014 11:11 AM, Pavel Stehule wrote:

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.

OK. it was so trivial I just did it.

I think it may not be quite as trivial as that. In particular, PSQLexec
already contains error-reporting functionality, so I think that the
minimal_error_message stuff may now be dead. You should hack things to
cause a query error in there and see if the reporting behavior is nice.

regards, tom lane

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

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#5)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

On 11/21/2014 12:32 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 11/21/2014 11:11 AM, Pavel Stehule wrote:

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.

OK. it was so trivial I just did it.

I think it may not be quite as trivial as that. In particular, PSQLexec
already contains error-reporting functionality, so I think that the
minimal_error_message stuff may now be dead. You should hack things to
cause a query error in there and see if the reporting behavior is nice.

Oh. ok.

cheers

andrew

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

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Andrew Dunstan (#6)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

On 11/21/2014 01:05 PM, Andrew Dunstan wrote:

On 11/21/2014 12:32 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 11/21/2014 11:11 AM, Pavel Stehule wrote:

I noticed the other day that psql doesn't honor ECHO_HIDDEN for \sf.

OK. it was so trivial I just did it.

I think it may not be quite as trivial as that. In particular, PSQLexec
already contains error-reporting functionality, so I think that the
minimal_error_message stuff may now be dead. You should hack things to
cause a query error in there and see if the reporting behavior is nice.

Oh. ok.

Well, now we get things like this:

ERROR: more than one function named "abc"
LINE 1: SELECT 'abc'::pg_catalog.regproc::pg_catalog.oid

whereas minimal_error_message suppressed the second line. If we want to
preserve that older behaviour we'll have to abandon use of PSQLexec. But
it's not so complex that that would be a huge issue.

cheers

andrew

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

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Andrew Dunstan (#7)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

Andrew Dunstan <andrew@dunslane.net> writes:

Well, now we get things like this:

ERROR: more than one function named "abc"
LINE 1: SELECT 'abc'::pg_catalog.regproc::pg_catalog.oid

whereas minimal_error_message suppressed the second line. If we want to
preserve that older behaviour we'll have to abandon use of PSQLexec. But
it's not so complex that that would be a huge issue.

Yeah, the reason why we wrote that code to begin with was that there
were a bunch of user-facing error cases that would be reported by
regproc_in/regprocedure_in, and we didn't want to clutter those error
reports with the underlying queries.

I'm not sure how I feel about changing this. Making these queries subject
to ECHO_HIDDEN seems like we're exposing them to users to some extent
anyway, and maybe it's not worth dozens of lines of code (and duplicating
large parts of PSQLexec) to avoid the extra output. On the other hand
this output doesn't seem very nice from a fit-and-finish standpoint.

regards, tom lane

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

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#8)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

On 11/21/2014 02:44 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Well, now we get things like this:
ERROR: more than one function named "abc"
LINE 1: SELECT 'abc'::pg_catalog.regproc::pg_catalog.oid
whereas minimal_error_message suppressed the second line. If we want to
preserve that older behaviour we'll have to abandon use of PSQLexec. But
it's not so complex that that would be a huge issue.

Yeah, the reason why we wrote that code to begin with was that there
were a bunch of user-facing error cases that would be reported by
regproc_in/regprocedure_in, and we didn't want to clutter those error
reports with the underlying queries.

I'm not sure how I feel about changing this. Making these queries subject
to ECHO_HIDDEN seems like we're exposing them to users to some extent
anyway, and maybe it's not worth dozens of lines of code (and duplicating
large parts of PSQLexec) to avoid the extra output. On the other hand
this output doesn't seem very nice from a fit-and-finish standpoint.

Let me see what I can come up with.

cheers

andrew

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

#10

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#8)

1 attachment(s)

Re: psql \sf doesn't show it's SQL when ECHO_HIDDEN is on

On 11/21/2014 02:44 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Well, now we get things like this:
ERROR: more than one function named "abc"
LINE 1: SELECT 'abc'::pg_catalog.regproc::pg_catalog.oid
whereas minimal_error_message suppressed the second line. If we want to
preserve that older behaviour we'll have to abandon use of PSQLexec. But
it's not so complex that that would be a huge issue.

Yeah, the reason why we wrote that code to begin with was that there
were a bunch of user-facing error cases that would be reported by
regproc_in/regprocedure_in, and we didn't want to clutter those error
reports with the underlying queries.

I'm not sure how I feel about changing this. Making these queries subject
to ECHO_HIDDEN seems like we're exposing them to users to some extent
anyway, and maybe it's not worth dozens of lines of code (and duplicating
large parts of PSQLexec) to avoid the extra output. On the other hand
this output doesn't seem very nice from a fit-and-finish standpoint.

Here's a patch that I think does what you want. I didn't have to borrow
too much code from PSQLexec(). Sample session:

andrew=# \sf abc
ERROR: more than one function named "abc"
andrew=# \sf blurfl
ERROR: function "blurfl" does not exist
andrew=# \sf abc()
CREATE OR REPLACE FUNCTION public.abc()
RETURNS integer
LANGUAGE sql
AS $function$ select 1$function$
andrew=# \set ECHO_HIDDEN 1
andrew=# \sf abc()
********* QUERY **********
SELECT 'abc()'::pg_catalog.regprocedure::pg_catalog.oid
**************************

********* QUERY **********
SELECT pg_catalog.pg_get_functiondef(16385)
**************************

CREATE OR REPLACE FUNCTION public.abc()
RETURNS integer
LANGUAGE sql
AS $function$ select 1$function$
andrew=# \sf abc
********* QUERY **********
SELECT 'abc'::pg_catalog.regproc::pg_catalog.oid
**************************

ERROR: more than one function named "abc"
andrew=#

cheers

andrew

Attachments:

psql-sf-ef-echo_hidden-fix.patchtext/x-diff; name=psql-sf-ef-echo_hidden-fix.patchDownload

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 36d5e36..162fcf1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2925,6 +2925,34 @@ do_watch(PQExpBuffer query_buf, long sleep)
 }
 
 /*
+ * a little code borrowed from PSQLexec() to manage ECHO_HIDDEN output.
+ * returns true unless we have ECHO_HIDDEN_NOEXEC.
+ */
+static bool
+lookup_function_echo_hidden(char * query)
+{
+	if (pset.echo_hidden != PSQL_ECHO_HIDDEN_OFF)
+	{
+		printf(_("********* QUERY **********\n"
+				 "%s\n"
+				 "**************************\n\n"), query);
+		fflush(stdout);
+		if (pset.logfile)
+		{
+			fprintf(pset.logfile,
+					_("********* QUERY **********\n"
+					  "%s\n"
+					  "**************************\n\n"), query);
+			fflush(pset.logfile);
+		}
+
+		if (pset.echo_hidden == PSQL_ECHO_HIDDEN_NOEXEC)
+			return false;
+	}
+	return true;
+}
+
+/*
  * This function takes a function description, e.g. "x" or "x(int)", and
  * issues a query on the given connection to retrieve the function's OID
  * using a cast to regproc or regprocedure (as appropriate). The result,
@@ -2945,8 +2973,9 @@ lookup_function_oid(const char *desc, Oid *foid)
 	appendStringLiteralConn(query, desc, pset.db);
 	appendPQExpBuffer(query, "::pg_catalog.%s::pg_catalog.oid",
 					  strchr(desc, '(') ? "regprocedure" : "regproc");
-
-	res = PSQLexec(query->data);
+	if (!lookup_function_echo_hidden(query->data))
+		return false;
+	res = PQexec(pset.db, query->data);
 	if (PQresultStatus(res) == PGRES_TUPLES_OK && PQntuples(res) == 1)
 		*foid = atooid(PQgetvalue(res, 0, 0));
 	else
@@ -2975,7 +3004,9 @@ get_create_function_cmd(Oid oid, PQExpBuffer buf)
 	query = createPQExpBuffer();
 	printfPQExpBuffer(query, "SELECT pg_catalog.pg_get_functiondef(%u)", oid);
 
-	res = PSQLexec(query->data);
+	if (!lookup_function_echo_hidden(query->data))
+		return false;
+	res = PQexec(pset.db, query->data);
 	if (PQresultStatus(res) == PGRES_TUPLES_OK && PQntuples(res) == 1)
 	{
 		resetPQExpBuffer(buf);