|From:||Bruce Momjian <bruce(at)momjian(dot)us>|
|To:||Julien Rouhaud <rjuju123(at)gmail(dot)com>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Hannu Krosing <hannuk(at)google(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, Atsushi Torikoshi <atorik(at)gmail(dot)com>, Tatsuro Yamada <tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Evgeny Efimkin <efimkin(at)yandex-team(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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 <bruce(at)momjian(dot)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
> 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.
If only the physical world exists, free will is an illusion.
|Next Message||Bruce Momjian||2021-04-08 01:00:40||Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?|
|Previous Message||Julien Rouhaud||2021-04-08 00:47:48||Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?|