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 <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?
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+g67SJoKY65Cg9Z1QOhSyhCEU8w3g@mail.gmail.com.
As far as I know this is OK, as we want to show consistent values everywhere.
> 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.
@@ -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.