From f0d11275954719fd5d0281d4135e5c78de46e099 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 12 Aug 2024 15:42:16 +1200 Subject: [PATCH] Remove "parent" column from pg_backend_memory_contexts 32d3ed816 added the "path" column to pg_backend_memory_contexts to allow a stable method of obtaining the parent MemoryContext of a given row in the view. Using the "path" column is now the preferred method of obtaining the parent row. Previously, any queries which were self-joining to this view using the "name" and "parent" columns could get incorrect results due to the fact that names are not unique. Here we aim to explicitly break such queries so that they can be corrected and use the "path" column instead. It is possible that there are more innocent users of the parent column that just need an indication of the parent and having to write out a self-joining CTE may be an unnecessary hassle for those cases. Let's remove the column for now and see if anyone comes back with any complaints. This does seem like a good time to attempt to get rid of the column as we still have around 1 year to revert this if someone comes back with a valid complaint. Plus this view is new to v14 and is quite niche, so perhaps not many people will be affected. Author: Melih Mutlu Discussion: https://postgr.es/m/CAGPVpCT7NOe4fZXRL8XaoxHpSXYTu6GTpULT_3E-HT9hzjoFRA@mail.gmail.com --- doc/src/sgml/system-views.sgml | 9 ---- src/backend/utils/adt/mcxtfuncs.c | 65 +++++++++----------------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 6 +-- src/test/regress/expected/rules.out | 3 +- src/test/regress/expected/sysviews.out | 16 +++---- src/test/regress/sql/sysviews.sql | 4 +- 7 files changed, 36 insertions(+), 69 deletions(-) diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml index a0b692bf1e9..634a4c0fab4 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -481,15 +481,6 @@ - - - parent text - - - Name of the parent of this memory context - - - type text diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c index 5905958c1f5..6a6634e1cda 100644 --- a/src/backend/utils/adt/mcxtfuncs.c +++ b/src/backend/utils/adt/mcxtfuncs.c @@ -40,28 +40,6 @@ typedef struct MemoryContextId int context_id; } MemoryContextId; -/* - * get_memory_context_name_and_ident - * Populate *name and *ident from the name and ident from 'context'. - */ -static void -get_memory_context_name_and_ident(MemoryContext context, const char **const name, - const char **const ident) -{ - *name = context->name; - *ident = context->ident; - - /* - * To be consistent with logging output, we label dynahash contexts with - * just the hash table name as with MemoryContextStatsPrint(). - */ - if (*ident == NULL && strcmp(*name, "dynahash") == 0) - { - *name = *ident; - *ident = NULL; - } -} - /* * int_list_to_array * Convert an IntList to an array of INT4OIDs. @@ -93,7 +71,7 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore, TupleDesc tupdesc, MemoryContext context, HTAB *context_id_lookup) { -#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 11 +#define PG_GET_BACKEND_MEMORY_CONTEXTS_COLS 10 Datum values[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS]; bool nulls[PG_GET_BACKEND_MEMORY_CONTEXTS_COLS]; @@ -128,7 +106,18 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore, memset(values, 0, sizeof(values)); memset(nulls, 0, sizeof(nulls)); - get_memory_context_name_and_ident(context, &name, &ident); + name = context->name; + ident = context->ident; + + /* + * To be consistent with logging output, we label dynahash contexts with + * just the hash table name as with MemoryContextStatsPrint(). + */ + if (ident && strcmp(name, "dynahash") == 0) + { + name = ident; + ident = NULL; + } if (name) values[0] = CStringGetTextDatum(name); @@ -154,18 +143,6 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore, else nulls[1] = true; - if (context->parent) - { - const char *parent_name, - *parent_ident; - - get_memory_context_name_and_ident(context->parent, &parent_name, - &parent_ident); - values[2] = CStringGetTextDatum(parent_name); - } - else - nulls[2] = true; - switch (context->type) { case T_AllocSetContext: @@ -185,14 +162,14 @@ PutMemoryContextsStatsTupleStore(Tuplestorestate *tupstore, break; } - values[3] = CStringGetTextDatum(type); - values[4] = Int32GetDatum(list_length(path)); /* level */ - values[5] = int_list_to_array(path); - values[6] = Int64GetDatum(stat.totalspace); - values[7] = Int64GetDatum(stat.nblocks); - values[8] = Int64GetDatum(stat.freespace); - values[9] = Int64GetDatum(stat.freechunks); - values[10] = Int64GetDatum(stat.totalspace - stat.freespace); + values[2] = CStringGetTextDatum(type); + values[3] = Int32GetDatum(list_length(path)); /* level */ + values[4] = int_list_to_array(path); + values[5] = Int64GetDatum(stat.totalspace); + values[6] = Int64GetDatum(stat.nblocks); + values[7] = Int64GetDatum(stat.freespace); + values[8] = Int64GetDatum(stat.freechunks); + values[9] = Int64GetDatum(stat.totalspace - stat.freespace); tuplestore_putvalues(tupstore, tupdesc, values, nulls); list_free(path); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 565d68acd3d..37c0354ab91 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202408021 +#define CATALOG_VERSION_NO 202408121 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index d36f6001bb1..be37576a34a 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -8329,9 +8329,9 @@ proname => 'pg_get_backend_memory_contexts', prorows => '100', proretset => 't', provolatile => 'v', proparallel => 'r', prorettype => 'record', proargtypes => '', - proallargtypes => '{text,text,text,text,int4,_int4,int8,int8,int8,int8,int8}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{name, ident, parent, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}', + proallargtypes => '{text,text,text,int4,_int4,int8,int8,int8,int8,int8}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o}', + proargnames => '{name, ident, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes}', prosrc => 'pg_get_backend_memory_contexts' }, # logging memory contexts of the specified backend diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 52012806699..862433ee52b 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1305,7 +1305,6 @@ pg_available_extensions| SELECT e.name, LEFT JOIN pg_extension x ON ((e.name = x.extname))); pg_backend_memory_contexts| SELECT name, ident, - parent, type, level, path, @@ -1314,7 +1313,7 @@ pg_backend_memory_contexts| SELECT name, free_bytes, free_chunks, used_bytes - FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, parent, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes); + FROM pg_get_backend_memory_contexts() pg_get_backend_memory_contexts(name, ident, type, level, path, total_bytes, total_nblocks, free_bytes, free_chunks, used_bytes); pg_config| SELECT name, setting FROM pg_config() pg_config(name, setting); diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out index 84b3b64b490..fad7fc3a7e0 100644 --- a/src/test/regress/expected/sysviews.out +++ b/src/test/regress/expected/sysviews.out @@ -21,11 +21,11 @@ select count(*) >= 0 as ok from pg_available_extensions; -- The entire output of pg_backend_memory_contexts is not stable, -- we test only the existence and basic condition of TopMemoryContext. -select type, name, ident, parent, level, total_bytes >= free_bytes +select type, name, ident, level, total_bytes >= free_bytes from pg_backend_memory_contexts where level = 1; - type | name | ident | parent | level | ?column? -----------+------------------+-------+--------+-------+---------- - AllocSet | TopMemoryContext | | | 1 | t + type | name | ident | level | ?column? +----------+------------------+-------+-------+---------- + AllocSet | TopMemoryContext | | 1 | t (1 row) -- We can exercise some MemoryContext type stats functions. Most of the @@ -43,11 +43,11 @@ fetch 1 from cur; bbbbbbbbbb | 2 (1 row) -select type, name, parent, total_bytes > 0, total_nblocks, free_bytes > 0, free_chunks +select type, name, total_bytes > 0, total_nblocks, free_bytes > 0, free_chunks from pg_backend_memory_contexts where name = 'Caller tuples'; - type | name | parent | ?column? | total_nblocks | ?column? | free_chunks -------+---------------+----------------+----------+---------------+----------+------------- - Bump | Caller tuples | TupleSort sort | t | 2 | t | 0 + type | name | ?column? | total_nblocks | ?column? | free_chunks +------+---------------+----------+---------------+----------+------------- + Bump | Caller tuples | t | 2 | t | 0 (1 row) rollback; diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql index 15e2a9e7417..b2a79237543 100644 --- a/src/test/regress/sql/sysviews.sql +++ b/src/test/regress/sql/sysviews.sql @@ -14,7 +14,7 @@ select count(*) >= 0 as ok from pg_available_extensions; -- The entire output of pg_backend_memory_contexts is not stable, -- we test only the existence and basic condition of TopMemoryContext. -select type, name, ident, parent, level, total_bytes >= free_bytes +select type, name, ident, level, total_bytes >= free_bytes from pg_backend_memory_contexts where level = 1; -- We can exercise some MemoryContext type stats functions. Most of the @@ -28,7 +28,7 @@ declare cur cursor for select left(a,10), b from (values(repeat('a', 512 * 1024),1),(repeat('b', 512),2)) v(a,b) order by v.a desc; fetch 1 from cur; -select type, name, parent, total_bytes > 0, total_nblocks, free_bytes > 0, free_chunks +select type, name, total_bytes > 0, total_nblocks, free_bytes > 0, free_chunks from pg_backend_memory_contexts where name = 'Caller tuples'; rollback; -- 2.30.2