From: Bruce Momjian <bruce@momjian.us>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Hannu Krosing <hannuk@google.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: torikoshia <torikoshia@oss.nttdata.com>
Cc: Atsushi Torikoshi <atorik@gmail.com>
Cc: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Evgeny Efimkin <efimkin@yandex-team.ru>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?
Date: Wed, 7 Apr 2021 19:38:35 -0400
Message-ID: <20210407233835.GF24239@momjian.us> (raw)
In-Reply-To: <1730584.1617836485@sss.pgh.pa.us>
References: <CAMm1aWZBBPPjRau9dP6+PZFoirKMvy3tX5uqrY-wF0PgR9KvSg@mail.gmail.com>
<20210406151102.b2ytqji4m6wfhraf@nol>
<CAMm1aWbTTc7AJrAZa7q1QgNNqdNkFP_-owKHDR=zEjQGqevnpg@mail.gmail.com>
<20210407125726.tkvjdbw76hxnpwfi@nol>
<20210407181211.GB24239@momjian.us>
<1646131.1617826550@sss.pgh.pa.us>
<20210407202255.GD24239@momjian.us>
<20210407215625.syzdpwew4j47hhgl@nol>
<20210407223840.GE24239@momjian.us>
<1730584.1617836485@sss.pgh.pa.us>
On Wed, Apr 7, 2021 at 07:01:25PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@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?
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.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
If only the physical world exists, free will is an illusion.
Attachments:csv.diff (text/x-diff, 2.8K) download
From 2d2bafb3c4c205ef5899074447f316d157345faf Mon Sep 17 00:00:00 2001
From: Bruce Momjian <bruce@momjian.us>
Date: Wed, 7 Apr 2021 19:29:09 -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.sgmlindex 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.sgmlindex 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.cindex 1cf71a649b..1ac18d9a55 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 */
@@ -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);
--
2.20.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bruce@momjian.us, tgl@sss.pgh.pa.us, rjuju123@gmail.com, nitinjadhavpostgres@gmail.com, alvherre@alvh.no-ip.org, hannuk@google.com, robertmhaas@gmail.com, pavel.stehule@gmail.com, michael@paquier.xyz, torikoshia@oss.nttdata.com, atorik@gmail.com, tatsuro.yamada.tf@nttcom.co.jp, tomas.vondra@2ndquadrant.com, efimkin@yandex-team.ru, pgsql-hackers@lists.postgresql.org
Subject: Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?
In-Reply-To: <20210407233835.GF24239@momjian.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox