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.
