On Thu, Apr 8, 2021 at 08:47:48AM +0800, Julien Rouhaud wrote:
> On Wed, Apr 07, 2021 at 07:38:35PM -0400, Bruce Momjian wrote:
> > On Wed, Apr 7, 2021 at 07:01:25PM -0400, Tom Lane wrote:
> > > Bruce Momjian <br...@momjian.us> writes:
> > > > Uh, I think your patch missed a few things. First, you use "%zd"
> > > > (size_t) for the printf string, but calls to pgstat_get_my_queryid() in
> > > > src/backend/utils/error/elog.c used "%ld". Which is correct? I see
> > > > pgstat_get_my_queryid() as returning uint64, but I didn't think a uint64
> > > > fits in a BIGINT SQL column.
> > >
> > > Neither is correct. Project standard these days for printing [u]int64
> > > is to write "%lld" or "%llu", with an explicit (long long) cast on
> > > the printf argument.
> >
> > Yep, got it. The attached patch fixes all the calls to use %lld, and
> > adds casts. In implementing cvslog, I noticed that internally we pass
> > the hash as uint64, but output as int64, which I think is a requirement
> > for how pg_stat_statements has output it, and the use of bigint. Is
> > that OK?
>
> Indeed, this is due to how we expose the value in SQL. The original
> discussion
> is at
> https://www.postgresql.org/message-id/cah2-wzkuemfamy3onoxli+g67sjoky65cg9z1qohsyhceu8...@mail.gmail.com.
> As far as I know this is OK, as we want to show consistent values everywhere.
OK, yes, I do remember the discussion. I was wondering if there should
be a C comment about this anywhere.
> > I am also confused about the inconsistency of calling the GUC
> > compute_query_id (with underscore), but pg_stat_activity.queryid. If we
> > make it pg_stat_activity.query_id, it doesn't match most of the other
> > *id columsns in the table, leader_pid, usesysid, backend_xid. Is that
> > OK?I know I suggested pg_stat_activity.query_id, but maybe I was wrong.
>
> Mmm, most of the columns in pg_stat_activity do have a "_", so using query_id
> would make more sense.
OK, let me work on a patch to change that part.
> @@ -2967,6 +2967,10 @@ write_csvlog(ErrorData *edata)
>
> appendStringInfoChar(&buf, '\n');
>
> + /* query id */
> + appendStringInfo(&buf, "%lld", (long long) pgstat_get_my_queryid());
> + appendStringInfoChar(&buf, ',');
> +
>
> /* If in the syslogger process, try to write messages direct to file */
> if (MyBackendType == B_LOGGER)
> write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_CSVLOG);
>
> Unless I'm missing something this will output the query id in the next log
> line? The new code should be added before the newline is output, and the
> comma
> should also be output before the queryid.
Yes, correct, updated patch attached.
--
Bruce Momjian <br...@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
>From 2e4beaef3690f7b61b91264a95a270e5410dad53 Mon Sep 17 00:00:00 2001
From: Bruce Momjian <br...@momjian.us>
Date: Wed, 7 Apr 2021 20:51:10 -0400
Subject: [PATCH] csv squash commit
---
doc/src/sgml/config.sgml | 4 +++-
doc/src/sgml/file-fdw.sgml | 3 ++-
src/backend/utils/error/elog.c | 12 ++++++++----
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 963824d050..ea5cf3a2dc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7310,7 +7310,8 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
character count of the error position therein,
location of the error in the PostgreSQL source code
(if <varname>log_error_verbosity</varname> is set to <literal>verbose</literal>),
- application name, backend type, and process ID of parallel group leader.
+ application name, backend type, process ID of parallel group leader,
+ and query id.
Here is a sample table definition for storing CSV-format log output:
<programlisting>
@@ -7341,6 +7342,7 @@ CREATE TABLE postgres_log
application_name text,
backend_type text,
leader_pid integer,
+ query_id bigint,
PRIMARY KEY (session_id, session_line_num)
);
</programlisting>
diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml
index 2e21806f48..5b98782064 100644
--- a/doc/src/sgml/file-fdw.sgml
+++ b/doc/src/sgml/file-fdw.sgml
@@ -266,7 +266,8 @@ CREATE FOREIGN TABLE pglog (
location text,
application_name text,
backend_type text,
- leader_pid integer
+ leader_pid integer,
+ query_id bigint
) SERVER pglog
OPTIONS ( filename 'log/pglog.csv', format 'csv' );
</programlisting>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 1cf71a649b..a1ebe06d5b 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2716,11 +2716,11 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
break;
case 'Q':
if (padding != 0)
- appendStringInfo(buf, "%*ld", padding,
- pgstat_get_my_queryid());
+ appendStringInfo(buf, "%*lld", padding,
+ (long long) pgstat_get_my_queryid());
else
- appendStringInfo(buf, "%ld",
- pgstat_get_my_queryid());
+ appendStringInfo(buf, "%lld",
+ (long long) pgstat_get_my_queryid());
break;
default:
/* format error - ignore it */
@@ -2964,6 +2964,10 @@ write_csvlog(ErrorData *edata)
if (leader && leader->pid != MyProcPid)
appendStringInfo(&buf, "%d", leader->pid);
}
+ appendStringInfoChar(&buf, ',');
+
+ /* query id */
+ appendStringInfo(&buf, "%lld", (long long) pgstat_get_my_queryid());
appendStringInfoChar(&buf, '\n');
--
2.20.1
