Hello Tom,
I went to commit this, figuring that it was a trivial bit of code
consolidation, but as I looked around in common.c I got rather
unhappy with the inconsistent behavior of things. Examining
the various places that implement "echo"-related logic, we have
the three places this patch proposes to unify, which log queries
using
fprintf(out,
_("********* QUERY **********\n"
"%s\n"
"**************************\n\n"), query);
and then we have two more that just do
puts(query);
plus this:
if (!OK && pset.echo == PSQL_ECHO_ERRORS)
pg_log_info("STATEMENT: %s", query);
So it's exactly fifty-fifty as to whether we add all that decoration
or none at all. I think if we're going to touch this logic, we
ought to try to unify the behavior.
+1.
I did not go this way because I wanted it to be a simple restructuring
patch so that it could go through without much ado, but I agree with
improving the current status. I'm not sure we want too much ascii-art.
My vote would be to drop the decoration everywhere, but perhaps there
are votes not to?
No, I'd be ok with removing the decoration, or at least simplify them, or
as you suggest below make the have a useful semantics.
A different angle is that the identical decoration is used for both
psql-generated queries that are logged because of ECHO_HIDDEN, and
user-entered queries. This seems at best rather unhelpful.
Indeed.
If we keep the decoration, should we make it different for those two
cases? (Maybe "INTERNAL QUERY" vs "QUERY", for example.) The cases
with no decoration likewise fall into multiple categories, both
user-entered and generated-by-gexec; if we were going with a decorated
approach I'd think it useful to make a distinction between those, too.
Thoughts?
Yes. Maybe decorations should be SQL comments, and the purpose/origin of
the query could be made clear as you suggest, eg something like markdown
in a comment:
"-- # <whatever> QUERY\n%s\n\n"
with <whatever> in USER DESCRIPTION COMPLETION GEXEC…
--
Fabien.
