mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove "parent" column from pg_backend_memory_contexts

Browse Source 
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 <m.melihmutlu@gmail.com>
Discussion: https://postgr.es/m/CAGPVpCT7NOe4fZXRL8XaoxHpSXYTu6GTpULT_3E-HT9hzjoFRA@mail.gmail.com
This commit is contained in:
David Rowley 2024-08-12 15:42:16 +12:00
parent 3f44959f47
commit f0d1127595
7 changed files with 36 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
doc/src/sgml/system-views.sgml
View File

@ -481,15 +481,6 @@
</para></entry>
</row>
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>parent</structfield> <type>text</type>
</para>
<para>
Name of the parent of this memory context
</para></entry>
</row>
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>type</structfield> <type>text</type>

65
src/backend/utils/adt/mcxtfuncs.c
View File

@ -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);

2
src/include/catalog/catversion.h
View File

@ -57,6 +57,6 @@
*/
/* yyyymmddN */
#define CATALOG_VERSION_NO 202408021
#define CATALOG_VERSION_NO 202408121
#endif

6
src/include/catalog/pg_proc.dat
View File

@ -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

3
src/test/regress/expected/rules.out
View File

@ -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);

16
src/test/regress/expected/sysviews.out
View File

@ -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;

4
src/test/regress/sql/sysviews.sql
View File

@ -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;