On Mon, Mar 15, 2021 at 03:10:59PM +0900, Michael Paquier wrote: > Anyway, as mentioned by other people upthread, I am not really > convinced either that we should have more flavors of size functions, > particularly depending on the relkind as this would be confusing for > the end-user. pg_relation_size() can already do this job for all > relkinds that use segment files, so it should still be able to hold > its ground and maintain a consistent behavior with what it does > currently.
On top of the rest of my notes, there are two more things that would face a behavior change if making the existing functions go through table AMs, which would scan the data in the smgr: - After a VACUUM, the relation would be reported with a size of 0, while that may not be the case of on-disk files yet. - Temporary tables of other sessions would be accessible. So we would likely want a separate function. Another possibility, which I find tempting, would be to push down the calculation logic relying on physical files down to the table AM themselves with a new dedicated callback (relation_size_physical?), as it seems to me that the most important thing users want to know with this set of functions is how much physical space is being consumed at one given point in time. Attached is a small prototype I was just playing with. -- Michael
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 64cdaa4134..8bd5d33265 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -15,6 +15,7 @@
#include "access/htup_details.h"
#include "access/relation.h"
+#include "access/tableam.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
#include "catalog/pg_authid.h"
@@ -23,6 +24,7 @@
#include "commands/tablespace.h"
#include "miscadmin.h"
#include "storage/fd.h"
+#include "storage/smgr.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/numeric.h"
@@ -270,14 +272,28 @@ pg_tablespace_size_name(PG_FUNCTION_ARGS)
* is no check here or at the call sites for that.
*/
static int64
-calculate_relation_size(RelFileNode *rfn, BackendId backend, ForkNumber forknum)
+calculate_relation_size(Relation rel, ForkNumber forknum)
{
int64 totalsize = 0;
char *relationpath;
char pathname[MAXPGPATH];
unsigned int segcount = 0;
- relationpath = relpathbackend(*rfn, backend, forknum);
+ /*
+ * If the relation is related to a table AM, do its sizing directly
+ * using its interface.
+ */
+ if (rel->rd_rel->relkind == RELKIND_RELATION ||
+ rel->rd_rel->relkind == RELKIND_TOASTVALUE ||
+ rel->rd_rel->relkind == RELKIND_MATVIEW)
+ {
+ if (rel->rd_smgr && smgrexists(rel->rd_smgr, forknum))
+ return table_relation_size(rel, forknum);
+ else
+ return 0;
+ }
+
+ relationpath = relpathbackend(rel->rd_node, rel->rd_backend, forknum);
for (segcount = 0;; segcount++)
{
@@ -327,7 +343,7 @@ pg_relation_size(PG_FUNCTION_ARGS)
if (rel == NULL)
PG_RETURN_NULL();
- size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
+ size = calculate_relation_size(rel,
forkname_to_number(text_to_cstring(forkName)));
relation_close(rel, AccessShareLock);
@@ -352,8 +368,7 @@ calculate_toast_table_size(Oid toastrelid)
/* toast heap size, including FSM and VM size */
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
- size += calculate_relation_size(&(toastRel->rd_node),
- toastRel->rd_backend, forkNum);
+ size += calculate_relation_size(toastRel, forkNum);
/* toast index size, including FSM and VM size */
indexlist = RelationGetIndexList(toastRel);
@@ -366,8 +381,7 @@ calculate_toast_table_size(Oid toastrelid)
toastIdxRel = relation_open(lfirst_oid(lc),
AccessShareLock);
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
- size += calculate_relation_size(&(toastIdxRel->rd_node),
- toastIdxRel->rd_backend, forkNum);
+ size += calculate_relation_size(toastIdxRel, forkNum);
relation_close(toastIdxRel, AccessShareLock);
}
@@ -395,8 +409,7 @@ calculate_table_size(Relation rel)
* heap size, including FSM and VM
*/
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
- size += calculate_relation_size(&(rel->rd_node), rel->rd_backend,
- forkNum);
+ size += calculate_relation_size(rel, forkNum);
/*
* Size of toast relation
@@ -434,9 +447,7 @@ calculate_indexes_size(Relation rel)
idxRel = relation_open(idxOid, AccessShareLock);
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
- size += calculate_relation_size(&(idxRel->rd_node),
- idxRel->rd_backend,
- forkNum);
+ size += calculate_relation_size(idxRel, forkNum);
relation_close(idxRel, AccessShareLock);
}
diff --git a/src/test/regress/expected/reloptions.out b/src/test/regress/expected/reloptions.out
index 44c130409f..4b3ec17989 100644
--- a/src/test/regress/expected/reloptions.out
+++ b/src/test/regress/expected/reloptions.out
@@ -103,7 +103,7 @@ INSERT INTO reloptions_test VALUES (1, NULL), (NULL, NULL);
ERROR: null value in column "i" of relation "reloptions_test" violates not-null constraint
DETAIL: Failing row contains (null, null).
VACUUM reloptions_test;
-SELECT pg_relation_size('reloptions_test') > 0;
+SELECT pg_relation_size('reloptions_test') = 0;
?column?
----------
t
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 90cea6caa8..e11ace9c3c 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -169,7 +169,7 @@ INSERT INTO vac_truncate_test VALUES (1, NULL), (NULL, NULL);
ERROR: null value in column "i" of relation "vac_truncate_test" violates not-null constraint
DETAIL: Failing row contains (null, null).
VACUUM (TRUNCATE FALSE) vac_truncate_test;
-SELECT pg_relation_size('vac_truncate_test') > 0;
+SELECT pg_relation_size('vac_truncate_test') = 0;
?column?
----------
t
diff --git a/src/test/regress/sql/reloptions.sql b/src/test/regress/sql/reloptions.sql
index cac5b0bcb0..2cd43c825c 100644
--- a/src/test/regress/sql/reloptions.sql
+++ b/src/test/regress/sql/reloptions.sql
@@ -62,7 +62,7 @@ CREATE TABLE reloptions_test(i INT NOT NULL, j text)
SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test'::regclass;
INSERT INTO reloptions_test VALUES (1, NULL), (NULL, NULL);
VACUUM reloptions_test;
-SELECT pg_relation_size('reloptions_test') > 0;
+SELECT pg_relation_size('reloptions_test') = 0;
SELECT reloptions FROM pg_class WHERE oid =
(SELECT reltoastrelid FROM pg_class
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 93fd258fc0..b1d0b2fb26 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -149,7 +149,7 @@ CREATE TABLE vac_truncate_test(i INT NOT NULL, j text)
WITH (vacuum_truncate=true, autovacuum_enabled=false);
INSERT INTO vac_truncate_test VALUES (1, NULL), (NULL, NULL);
VACUUM (TRUNCATE FALSE) vac_truncate_test;
-SELECT pg_relation_size('vac_truncate_test') > 0;
+SELECT pg_relation_size('vac_truncate_test') = 0;
VACUUM vac_truncate_test;
SELECT pg_relation_size('vac_truncate_test') = 0;
VACUUM (TRUNCATE FALSE, FULL TRUE) vac_truncate_test;
signature.asc
Description: PGP signature
