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?

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  <br...@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

>From 2d2bafb3c4c205ef5899074447f316d157345faf Mon Sep 17 00:00:00 2001
From: Bruce Momjian <br...@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.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..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 via email to