public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 3/4] Use correct 'statistics kind' in a couple places
3+ messages / 3 participants
[nested] [flat]
* [PATCH 3/4] Use correct 'statistics kind' in a couple places
@ 2021-03-07 00:38 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tomas Vondra @ 2021-03-07 00:38 UTC (permalink / raw)
A couple places used 'statistic kind' which is inconsistent, so use
'statistics kind' consistently.
---
doc/src/sgml/catalogs.sgml | 2 +-
src/backend/statistics/dependencies.c | 2 +-
src/backend/statistics/extended_stats.c | 2 +-
src/backend/statistics/mcv.c | 2 +-
src/backend/statistics/mvdistinct.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index b1de6d0674..64601d6b24 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7358,7 +7358,7 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<structfield>stxkind</structfield> <type>char[]</type>
</para>
<para>
- An array containing codes for the enabled statistic kinds;
+ An array containing codes for the enabled statistics kinds;
valid values are:
<literal>d</literal> for n-distinct statistics,
<literal>f</literal> for functional dependency statistics, and
diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index f6e399b192..eac9285165 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -639,7 +639,7 @@ statext_dependencies_load(Oid mvoid)
Anum_pg_statistic_ext_data_stxddependencies, &isnull);
if (isnull)
elog(ERROR,
- "requested statistic kind \"%c\" is not yet built for statistics object %u",
+ "requested statistics kind \"%c\" is not yet built for statistics object %u",
STATS_EXT_DEPENDENCIES, mvoid);
result = statext_dependencies_deserialize(DatumGetByteaPP(deps));
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index a030ea3653..8c05e10d0c 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -64,7 +64,7 @@ typedef struct StatExtEntry
char *schema; /* statistics object's schema */
char *name; /* statistics object's name */
Bitmapset *columns; /* attribute numbers covered by the object */
- List *types; /* 'char' list of enabled statistic kinds */
+ List *types; /* 'char' list of enabled statistics kinds */
int stattarget; /* statistics target (-1 for default) */
} StatExtEntry;
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index abbc1f1ba8..8335dff241 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -570,7 +570,7 @@ statext_mcv_load(Oid mvoid)
if (isnull)
elog(ERROR,
- "requested statistic kind \"%c\" is not yet built for statistics object %u",
+ "requested statistics kind \"%c\" is not yet built for statistics object %u",
STATS_EXT_DEPENDENCIES, mvoid);
result = statext_mcv_deserialize(DatumGetByteaP(mcvlist));
diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c
index 9ef21debb6..e08c001e3f 100644
--- a/src/backend/statistics/mvdistinct.c
+++ b/src/backend/statistics/mvdistinct.c
@@ -153,7 +153,7 @@ statext_ndistinct_load(Oid mvoid)
Anum_pg_statistic_ext_data_stxdndistinct, &isnull);
if (isnull)
elog(ERROR,
- "requested statistic kind \"%c\" is not yet built for statistics object %u",
+ "requested statistics kind \"%c\" is not yet built for statistics object %u",
STATS_EXT_NDISTINCT, mvoid);
result = statext_ndistinct_deserialize(DatumGetByteaPP(ndist));
--
2.26.2
--------------BCA1373AE29B32B57608F4EE
Content-Type: text/x-patch; charset=UTF-8;
name="0004-Extended-statistics-on-expressions-20210307.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0004-Extended-statistics-on-expressions-20210307.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* pg_stat_get_backend_subxact() and backend IDs?
@ 2023-08-24 01:22 Ian Lawrence Barwick <[email protected]>
2023-08-24 02:32 ` Re: pg_stat_get_backend_subxact() and backend IDs? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Ian Lawrence Barwick @ 2023-08-24 01:22 UTC (permalink / raw)
To: pgsql-hackers
Hi
I was playing around with "pg_stat_get_backend_subxact()" (commit 10ea0f924)
and see it emits NULL values for some backends, e.g.:
postgres=# \pset null NULL
Null display is "NULL".
postgres=# SELECT id, pg_stat_get_backend_pid(id), s.*,
pg_stat_get_backend_activity (id)
FROM pg_stat_get_backend_idset() id
JOIN LATERAL pg_stat_get_backend_subxact(id) AS s ON TRUE;
id | pg_stat_get_backend_pid | subxact_count |
subxact_overflowed | pg_stat_get_backend_activity
-----+-------------------------+---------------+--------------------+------------------------------------------------------------
1 | 3175972 | 0 | f
| <command string not enabled>
2 | 3175973 | 0 | f
| <command string not enabled>
3 | 3177889 | 0 | f
| SELECT id, pg_stat_get_backend_pid(id), s.*, +
| | |
| pg_stat_get_backend_activity (id) +
| | |
| FROM pg_stat_get_backend_idset() id +
| | |
| JOIN LATERAL pg_stat_get_backend_subxact(id) AS s ON TRUE;
4 | 3176027 | 5 | f
| savepoint s4;
256 | 3175969 | NULL | NULL
| <command string not enabled>
258 | 3175968 | NULL | NULL
| <command string not enabled>
259 | 3175971 | NULL | NULL
| <command string not enabled>
(7 rows)
Reading through the thread [1], it looks like 0/false are intended to be
returned for non-backend processes too [2], so it seems odd that NULL/NULL is
getting returned in some cases, especially as that's what's returned if a
non-existent backend ID is provided.
[1] https://www.postgresql.org/message-id/flat/CAFiTN-uvYAofNRaGF4R%2Bu6_OrABdkqNRoX7V6%2BPP3H_0HuYMwg%4...
[2] https://www.postgresql.org/message-id/flat/CAFiTN-ut0uwkRJDQJeDPXpVyTWD46m3gt3JDToE02hTfONEN%3DQ%40m...
Looking at the code, this is happening because
"pgstat_fetch_stat_local_beentry()"
expects to be passed the backend ID as an integer representing a 1-based index
referring to "localBackendStatusTable", but "pg_stat_get_backend_subxact()"
is presumably intended to take the actual BackendId , as per other
"pg_stat_get_XXX()"
functions.
Also, the comment for "pgstat_fetch_stat_local_beentry()" says:
Returns NULL if the argument is out of range (no current caller does that).
so the last part is currently incorrect.
Assuming I am not misunderstanding something here (always a
possibility, apologies
in advance if this is merely noise), what is actually needed is a function which
accepts a BackendId (as per "pgstat_fetch_stat_beentry()"), but returns a
LocalPgBackendStatus (as per "pgstat_fetch_stat_local_beentry()") like the
attached, clumsily named "pgstat_fetch_stat_backend_local_beentry()".
Regards
Ian Barwick
Attachments:
[text/x-patch] pg_stat_get_backend_subxact-fix.v1.patch (2.5K, ../../CAB8KJ=j-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few@mail.gmail.com/2-pg_stat_get_backend_subxact-fix.v1.patch)
download | inline diff:
commit d4292d3347658f61855fd84827954f587838a324
Author: Ian Barwick <[email protected]>
Date: Thu Aug 24 10:16:51 2023 +0900
pg_stat_get_backend_subxact(): handle backend ID correctly
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 38f91a495b..bce7836264 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -1136,6 +1136,39 @@ pgstat_fetch_stat_local_beentry(int beid)
}
+/* ----------
+ * pgstat_fetch_stat_backend_local_beentry() -
+ *
+ * Like pgstat_fetch_stat_local_beentry() but takes the BackendId of the
+ * desired session.
+ *
+ * Returns NULL if the given beid doesn't identify any known session.
+ *
+ * NB: caller is responsible for a check if the user is permitted to see
+ * this info (especially the querystring).
+ * ----------
+ */
+
+LocalPgBackendStatus *
+pgstat_fetch_stat_backend_local_beentry(BackendId beid)
+{
+ LocalPgBackendStatus key;
+
+ pgstat_read_current_status();
+
+ /*
+ * Since the localBackendStatusTable is in order by backend_id, we can use
+ * bsearch() to search it efficiently.
+ */
+ key.backend_id = beid;
+
+ return (LocalPgBackendStatus *) bsearch(&key, localBackendStatusTable,
+ localNumBackends,
+ sizeof(LocalPgBackendStatus),
+ cmp_lbestatus);
+}
+
+
/* ----------
* pgstat_fetch_stat_numbackends() -
*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2b9742ad21..8479ea130b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -727,7 +727,7 @@ pg_stat_get_backend_subxact(PG_FUNCTION_ARGS)
BlessTupleDesc(tupdesc);
- if ((local_beentry = pgstat_fetch_stat_local_beentry(beid)) != NULL)
+ if ((local_beentry = pgstat_fetch_stat_backend_local_beentry(beid)) != NULL)
{
/* Fill values and NULLs */
values[0] = Int32GetDatum(local_beentry->backend_subxact_count);
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 77939a0aed..760ddc48ae 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -334,6 +334,7 @@ extern uint64 pgstat_get_my_query_id(void);
*/
extern int pgstat_fetch_stat_numbackends(void);
extern PgBackendStatus *pgstat_fetch_stat_beentry(BackendId beid);
+extern LocalPgBackendStatus *pgstat_fetch_stat_backend_local_beentry(BackendId beid);
extern LocalPgBackendStatus *pgstat_fetch_stat_local_beentry(int beid);
extern char *pgstat_clip_activity(const char *raw_activity);
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pg_stat_get_backend_subxact() and backend IDs?
2023-08-24 01:22 pg_stat_get_backend_subxact() and backend IDs? Ian Lawrence Barwick <[email protected]>
@ 2023-08-24 02:32 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Nathan Bossart @ 2023-08-24 02:32 UTC (permalink / raw)
To: Ian Lawrence Barwick <[email protected]>; +Cc: pgsql-hackers
On Thu, Aug 24, 2023 at 10:22:49AM +0900, Ian Lawrence Barwick wrote:
> Looking at the code, this is happening because
> "pgstat_fetch_stat_local_beentry()"
> expects to be passed the backend ID as an integer representing a 1-based index
> referring to "localBackendStatusTable", but "pg_stat_get_backend_subxact()"
> is presumably intended to take the actual BackendId , as per other
> "pg_stat_get_XXX()"
> functions.
Yes, this was changed in d7e39d7, but 10ea0f9 seems to have missed the
memo.
> Assuming I am not misunderstanding something here (always a
> possibility, apologies
> in advance if this is merely noise), what is actually needed is a function which
> accepts a BackendId (as per "pgstat_fetch_stat_beentry()"), but returns a
> LocalPgBackendStatus (as per "pgstat_fetch_stat_local_beentry()") like the
> attached, clumsily named "pgstat_fetch_stat_backend_local_beentry()".
I think you are right. The relevant information is only available in
LocalPgBackendStatus, but there's presently no helper function for
obtaining the "local" status with the BackendId.
> +LocalPgBackendStatus *
> +pgstat_fetch_stat_backend_local_beentry(BackendId beid)
> +{
> + LocalPgBackendStatus key;
> +
> + pgstat_read_current_status();
> +
> + /*
> + * Since the localBackendStatusTable is in order by backend_id, we can use
> + * bsearch() to search it efficiently.
> + */
> + key.backend_id = beid;
> +
> + return (LocalPgBackendStatus *) bsearch(&key, localBackendStatusTable,
> + localNumBackends,
> + sizeof(LocalPgBackendStatus),
> + cmp_lbestatus);
> +}
We could probably modify pgstat_fetch_stat_beentry() to use this new
function. I suspect we'll want to work on the naming, too. Maybe we could
name them pg_stat_fetch_local_beentry_by_index() and
pg_stat_fetch_local_beentry_by_backendid().
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-08-24 02:32 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-07 00:38 [PATCH 3/4] Use correct 'statistics kind' in a couple places Tomas Vondra <[email protected]>
2023-08-24 01:22 pg_stat_get_backend_subxact() and backend IDs? Ian Lawrence Barwick <[email protected]>
2023-08-24 02:32 ` Re: pg_stat_get_backend_subxact() and backend IDs? Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox