WIP patch: Improve relation size functions such as pg_relation_size() to avoid producing an error when called against a no longer visible relation
Attached is a patch that addresses the todo item "Improve relation
size functions such as pg_relation_size() to avoid producing an error
when called against a no longer visible relation."
http://archives.postgresql.org/pgsql-general/2010-10/msg00332.php
Instead of returning an error, they now return NULL if the OID is
found in pg_class when using SnapshotAny. I only applied it to 4
functions: select pg_relation_size, pg_total_relation_size,
pg_table_size and pg_indexes_size. These are the ones that were using
relation_open(). I changed them to using try_relation_open(). For
three of them I had to move the try_relation_open() call up one level
in the call stack and change the parameter types for some support
functions from Oid to Relation. They now also call a new function
relation_recently_dead() which is what checks for relation in
SnapshotAny. All regression tests passed.
Is SnapshotAny the snapshot I should be using? It seems to get the
correct results. I can drop a table and I get NULL. Then after a
vacuumdb it returns an error.
Attachments:
improve_relation_size_functions.patchtext/x-patch; charset=US-ASCII; name=improve_relation_size_functions.patchDownload
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
new file mode 100644
index 2ee5966..0623935
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
***************
*** 24,32 ****
--- 24,34 ----
#include "storage/fd.h"
#include "utils/acl.h"
#include "utils/builtins.h"
+ #include "utils/fmgroids.h"
#include "utils/rel.h"
#include "utils/relmapper.h"
#include "utils/syscache.h"
+ #include "utils/tqual.h"
/* Return physical size of directory contents, or 0 if dir doesn't exist */
*************** calculate_relation_size(RelFileNode *rfn
*** 281,286 ****
--- 283,321 ----
return totalsize;
}
+ /*
+ * relation_recently_dead
+ *
+ * Check to see if a relation exists in SnapshotAny
+ */
+ static bool
+ relation_recently_dead(Oid relOid)
+ {
+ Relation classRel;
+ ScanKeyData key[1];
+ HeapScanDesc scan;
+ bool status;
+
+ classRel = heap_open(RelationRelationId, AccessShareLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_class_relfilenode,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(relOid));
+
+ scan = heap_beginscan(classRel, SnapshotAny, 1, key);
+
+ if (heap_getnext(scan, ForwardScanDirection) != NULL)
+ status = true;
+ else
+ status = false;
+
+ heap_endscan(scan);
+ heap_close(classRel, AccessShareLock);
+
+ return status;
+ }
+
Datum
pg_relation_size(PG_FUNCTION_ARGS)
{
*************** pg_relation_size(PG_FUNCTION_ARGS)
*** 289,295 ****
Relation rel;
int64 size;
! rel = relation_open(relOid, AccessShareLock);
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
--- 324,338 ----
Relation rel;
int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if ((relOid != 0) && relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
*************** calculate_toast_table_size(Oid toastreli
*** 339,352 ****
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
ForkNumber forkNum;
- rel = relation_open(relOid, AccessShareLock);
-
/*
* heap size, including FSM and VM
*/
--- 382,392 ----
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Relation rel)
{
int64 size = 0;
ForkNumber forkNum;
/*
* heap size, including FSM and VM
*/
*************** calculate_table_size(Oid relOid)
*** 360,367 ****
if (OidIsValid(rel->rd_rel->reltoastrelid))
size += calculate_toast_table_size(rel->rd_rel->reltoastrelid);
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 400,405 ----
*************** calculate_table_size(Oid relOid)
*** 371,382 ****
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
-
- rel = relation_open(relOid, AccessShareLock);
/*
* Aggregate all indexes on the given relation
--- 409,417 ----
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Relation rel)
{
int64 size = 0;
/*
* Aggregate all indexes on the given relation
*************** calculate_indexes_size(Oid relOid)
*** 405,412 ****
list_free(index_oids);
}
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 440,445 ----
*************** Datum
*** 414,429 ****
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_table_size(relOid));
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_indexes_size(relOid));
}
/*
--- 447,494 ----
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if ((relOid != 0) && relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
!
! size = calculate_table_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if ((relOid != 0) && relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
!
! size = calculate_indexes_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
*************** pg_indexes_size(PG_FUNCTION_ARGS)
*** 431,437 ****
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Oid Relid)
{
int64 size;
--- 496,502 ----
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Relation Rel)
{
int64 size;
*************** calculate_total_relation_size(Oid Relid)
*** 439,450 ****
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Relid);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Relid);
return size;
}
--- 504,515 ----
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Rel);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Rel);
return size;
}
*************** calculate_total_relation_size(Oid Relid)
*** 452,460 ****
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_total_relation_size(relid));
}
/*
--- 517,541 ----
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relOid = PG_GETARG_OID(0);
! Relation rel;
! int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if ((relOid != 0) && relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
!
! size = calculate_total_relation_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
On Sat, Dec 17, 2011 at 3:52 PM, Phil Sorber <phil@omniti.com> wrote:
Attached is a patch that addresses the todo item "Improve relation
size functions such as pg_relation_size() to avoid producing an error
when called against a no longer visible relation."http://archives.postgresql.org/pgsql-general/2010-10/msg00332.php
Instead of returning an error, they now return NULL if the OID is
found in pg_class when using SnapshotAny. I only applied it to 4
functions: select pg_relation_size, pg_total_relation_size,
pg_table_size and pg_indexes_size. These are the ones that were using
relation_open(). I changed them to using try_relation_open(). For
three of them I had to move the try_relation_open() call up one level
in the call stack and change the parameter types for some support
functions from Oid to Relation. They now also call a new function
relation_recently_dead() which is what checks for relation in
SnapshotAny. All regression tests passed.Is SnapshotAny the snapshot I should be using? It seems to get the
correct results. I can drop a table and I get NULL. Then after a
vacuumdb it returns an error.
Something I'd like to point out myself is that I need to be using
ObjectIdAttributeNumber instead of Anum_pg_class_relfilenode. Probably
just luck that I got the intended results before. This will also
change the logic in a few other places.
Still not clear on the semantics of Snapshot{Any|Self|Now|Dirty}.
On Mon, Dec 19, 2011 at 1:27 PM, Phil Sorber <phil@omniti.com> wrote:
On Sat, Dec 17, 2011 at 3:52 PM, Phil Sorber <phil@omniti.com> wrote:
Attached is a patch that addresses the todo item "Improve relation
size functions such as pg_relation_size() to avoid producing an error
when called against a no longer visible relation."http://archives.postgresql.org/pgsql-general/2010-10/msg00332.php
Instead of returning an error, they now return NULL if the OID is
found in pg_class when using SnapshotAny. I only applied it to 4
functions: select pg_relation_size, pg_total_relation_size,
pg_table_size and pg_indexes_size. These are the ones that were using
relation_open(). I changed them to using try_relation_open(). For
three of them I had to move the try_relation_open() call up one level
in the call stack and change the parameter types for some support
functions from Oid to Relation. They now also call a new function
relation_recently_dead() which is what checks for relation in
SnapshotAny. All regression tests passed.Is SnapshotAny the snapshot I should be using? It seems to get the
correct results. I can drop a table and I get NULL. Then after a
vacuumdb it returns an error.Something I'd like to point out myself is that I need to be using
ObjectIdAttributeNumber instead of Anum_pg_class_relfilenode. Probably
just luck that I got the intended results before. This will also
change the logic in a few other places.Still not clear on the semantics of Snapshot{Any|Self|Now|Dirty}.
I've attached the updated version of my patch with the changes
mentioned in the previous email.
Attachments:
improve_relation_size_functions_v2.patchtext/x-patch; charset=US-ASCII; name=improve_relation_size_functions_v2.patchDownload
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
new file mode 100644
index 2ee5966..bc2c862
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
***************
*** 15,20 ****
--- 15,21 ----
#include <sys/stat.h>
#include "access/heapam.h"
+ #include "access/sysattr.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
#include "catalog/pg_tablespace.h"
***************
*** 24,32 ****
--- 25,35 ----
#include "storage/fd.h"
#include "utils/acl.h"
#include "utils/builtins.h"
+ #include "utils/fmgroids.h"
#include "utils/rel.h"
#include "utils/relmapper.h"
#include "utils/syscache.h"
+ #include "utils/tqual.h"
/* Return physical size of directory contents, or 0 if dir doesn't exist */
*************** calculate_relation_size(RelFileNode *rfn
*** 281,286 ****
--- 284,322 ----
return totalsize;
}
+ /*
+ * relation_recently_dead
+ *
+ * Check to see if a relation exists in SnapshotAny
+ */
+ static bool
+ relation_recently_dead(Oid relOid)
+ {
+ Relation classRel;
+ ScanKeyData key[1];
+ HeapScanDesc scan;
+ bool status;
+
+ classRel = heap_open(RelationRelationId, AccessShareLock);
+
+ ScanKeyInit(&key[0],
+ ObjectIdAttributeNumber,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(relOid));
+
+ scan = heap_beginscan(classRel, SnapshotAny, 1, key);
+
+ if (heap_getnext(scan, ForwardScanDirection) != NULL)
+ status = true;
+ else
+ status = false;
+
+ heap_endscan(scan);
+ heap_close(classRel, AccessShareLock);
+
+ return status;
+ }
+
Datum
pg_relation_size(PG_FUNCTION_ARGS)
{
*************** pg_relation_size(PG_FUNCTION_ARGS)
*** 289,295 ****
Relation rel;
int64 size;
! rel = relation_open(relOid, AccessShareLock);
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
--- 325,339 ----
Relation rel;
int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if (relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
*************** calculate_toast_table_size(Oid toastreli
*** 339,352 ****
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
ForkNumber forkNum;
- rel = relation_open(relOid, AccessShareLock);
-
/*
* heap size, including FSM and VM
*/
--- 383,393 ----
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Relation rel)
{
int64 size = 0;
ForkNumber forkNum;
/*
* heap size, including FSM and VM
*/
*************** calculate_table_size(Oid relOid)
*** 360,367 ****
if (OidIsValid(rel->rd_rel->reltoastrelid))
size += calculate_toast_table_size(rel->rd_rel->reltoastrelid);
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 401,406 ----
*************** calculate_table_size(Oid relOid)
*** 371,382 ****
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
-
- rel = relation_open(relOid, AccessShareLock);
/*
* Aggregate all indexes on the given relation
--- 410,418 ----
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Relation rel)
{
int64 size = 0;
/*
* Aggregate all indexes on the given relation
*************** calculate_indexes_size(Oid relOid)
*** 405,412 ****
list_free(index_oids);
}
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 441,446 ----
*************** Datum
*** 414,429 ****
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_table_size(relOid));
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_indexes_size(relOid));
}
/*
--- 448,495 ----
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if (relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
!
! size = calculate_table_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if (relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
!
! size = calculate_indexes_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
*************** pg_indexes_size(PG_FUNCTION_ARGS)
*** 431,437 ****
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Oid Relid)
{
int64 size;
--- 497,503 ----
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Relation Rel)
{
int64 size;
*************** calculate_total_relation_size(Oid Relid)
*** 439,450 ****
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Relid);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Relid);
return size;
}
--- 505,516 ----
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Rel);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Rel);
return size;
}
*************** calculate_total_relation_size(Oid Relid)
*** 452,460 ****
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_total_relation_size(relid));
}
/*
--- 518,542 ----
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relOid = PG_GETARG_OID(0);
! Relation rel;
! int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! {
! if (relation_recently_dead(relOid))
! PG_RETURN_NULL();
! else
! elog(ERROR, "could not open relation with OID %u", relOid);
! }
!
! size = calculate_total_relation_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
On Sat, Dec 17, 2011 at 3:52 PM, Phil Sorber <phil@omniti.com> wrote:
Is SnapshotAny the snapshot I should be using? It seems to get the
correct results. I can drop a table and I get NULL. Then after a
vacuumdb it returns an error.
The suggestion on the original thread was to use SnapshotDirty or the
caller's MVCC snapshot. SnapshotAny doesn't seem like a good idea.
We don't want to include rows from, say, transactions that have rolled
back. And SnapshotAny rows can stick around for a long time - weeks,
months. If a table is only occasionally updated, autovacuum may not
process it for a long time.
On the other hand, I think using SnapshotDirty is going to work out so
well: isn't there a race condition? The caller's MVCC snapshot seems
better, but I still see pitfalls. Who is to say that the immediate
caller has the same snapshot as the scan? I'm thinking of cases
involving things like CTEs and nested function calls.
I'm wondering if we oughta just return NULL and be done with it. If
SELECT select 1241241241::regclass doesn't choke, I'm not sure select
pg_relation_size(1241241241) ought to either. It's not like the user
is going to see NULL and have no clue that the relation wasn't found.
At the very worst they might think it's empty.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
I'm wondering if we oughta just return NULL and be done with it.
+1. There are multiple precedents for that sort of response, which we
introduced exactly so that "SELECT some_function(oid) FROM some_catalog"
wouldn't fail just because one of the rows had gotten deleted by the
time the scan got to it. I don't think it's necessary for the
relation-size functions to be any smarter. Indeed, I'd assumed that's
all that Phil's patch did, since I'd not looked closer till just now.
regards, tom lane
On Thu, Dec 22, 2011 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I'm wondering if we oughta just return NULL and be done with it.
+1. There are multiple precedents for that sort of response, which we
introduced exactly so that "SELECT some_function(oid) FROM some_catalog"
wouldn't fail just because one of the rows had gotten deleted by the
time the scan got to it. I don't think it's necessary for the
relation-size functions to be any smarter. Indeed, I'd assumed that's
all that Phil's patch did, since I'd not looked closer till just now.regards, tom lane
Here it is without the checking for recently dead. If it can't open
the relation it simply returns NULL.
Attachments:
improve_relation_size_functions_v3.patchtext/x-patch; charset=US-ASCII; name=improve_relation_size_functions_v3.patchDownload
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
new file mode 100644
index 2ee5966..134bc03
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
*************** pg_relation_size(PG_FUNCTION_ARGS)
*** 289,295 ****
Relation rel;
int64 size;
! rel = relation_open(relOid, AccessShareLock);
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
--- 289,298 ----
Relation rel;
int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
*************** calculate_toast_table_size(Oid toastreli
*** 339,352 ****
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
ForkNumber forkNum;
- rel = relation_open(relOid, AccessShareLock);
-
/*
* heap size, including FSM and VM
*/
--- 342,352 ----
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Relation rel)
{
int64 size = 0;
ForkNumber forkNum;
/*
* heap size, including FSM and VM
*/
*************** calculate_table_size(Oid relOid)
*** 360,367 ****
if (OidIsValid(rel->rd_rel->reltoastrelid))
size += calculate_toast_table_size(rel->rd_rel->reltoastrelid);
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 360,365 ----
*************** calculate_table_size(Oid relOid)
*** 371,382 ****
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
-
- rel = relation_open(relOid, AccessShareLock);
/*
* Aggregate all indexes on the given relation
--- 369,377 ----
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Relation rel)
{
int64 size = 0;
/*
* Aggregate all indexes on the given relation
*************** calculate_indexes_size(Oid relOid)
*** 405,412 ****
list_free(index_oids);
}
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 400,405 ----
*************** Datum
*** 414,429 ****
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_table_size(relOid));
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_indexes_size(relOid));
}
/*
--- 407,444 ----
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
!
! size = calculate_table_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
!
! size = calculate_indexes_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
*************** pg_indexes_size(PG_FUNCTION_ARGS)
*** 431,437 ****
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Oid Relid)
{
int64 size;
--- 446,452 ----
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Relation Rel)
{
int64 size;
*************** calculate_total_relation_size(Oid Relid)
*** 439,450 ****
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Relid);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Relid);
return size;
}
--- 454,465 ----
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Rel);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Rel);
return size;
}
*************** calculate_total_relation_size(Oid Relid)
*** 452,460 ****
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_total_relation_size(relid));
}
/*
--- 467,486 ----
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relOid = PG_GETARG_OID(0);
! Relation rel;
! int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
!
! size = calculate_total_relation_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
On Thu, Dec 22, 2011 at 2:02 PM, Phil Sorber <phil@omniti.com> wrote:
On Thu, Dec 22, 2011 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I'm wondering if we oughta just return NULL and be done with it.
+1. There are multiple precedents for that sort of response, which we
introduced exactly so that "SELECT some_function(oid) FROM some_catalog"
wouldn't fail just because one of the rows had gotten deleted by the
time the scan got to it. I don't think it's necessary for the
relation-size functions to be any smarter. Indeed, I'd assumed that's
all that Phil's patch did, since I'd not looked closer till just now.Here it is without the checking for recently dead. If it can't open
the relation it simply returns NULL.
I think we probably ought to make pg_database_size() and
pg_tablespace_size() behave similarly.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Dec 22, 2011 at 3:19 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Dec 22, 2011 at 2:02 PM, Phil Sorber <phil@omniti.com> wrote:
On Thu, Dec 22, 2011 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I'm wondering if we oughta just return NULL and be done with it.
+1. There are multiple precedents for that sort of response, which we
introduced exactly so that "SELECT some_function(oid) FROM some_catalog"
wouldn't fail just because one of the rows had gotten deleted by the
time the scan got to it. I don't think it's necessary for the
relation-size functions to be any smarter. Indeed, I'd assumed that's
all that Phil's patch did, since I'd not looked closer till just now.Here it is without the checking for recently dead. If it can't open
the relation it simply returns NULL.I think we probably ought to make pg_database_size() and
pg_tablespace_size() behave similarly.--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Changes added.
Attachments:
improve_relation_size_functions_v4.patchtext/x-patch; charset=US-ASCII; name=improve_relation_size_functions_v4.patchDownload
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
new file mode 100644
index 2ee5966..8c30dc4
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
*************** calculate_database_size(Oid dbOid)
*** 120,131 ****
FreeDir(dirdesc);
- /* Complain if we found no trace of the DB at all */
- if (!totalsize)
- ereport(ERROR,
- (ERRCODE_UNDEFINED_DATABASE,
- errmsg("database with OID %u does not exist", dbOid)));
-
return totalsize;
}
--- 120,125 ----
*************** Datum
*** 133,140 ****
pg_database_size_oid(PG_FUNCTION_ARGS)
{
Oid dbOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_database_size(dbOid));
}
Datum
--- 127,140 ----
pg_database_size_oid(PG_FUNCTION_ARGS)
{
Oid dbOid = PG_GETARG_OID(0);
+ int64 size;
! size = calculate_database_size(dbOid);
!
! if (!size)
! PG_RETURN_NULL();
!
! PG_RETURN_INT64(size);
}
Datum
*************** pg_database_size_name(PG_FUNCTION_ARGS)
*** 142,149 ****
{
Name dbName = PG_GETARG_NAME(0);
Oid dbOid = get_database_oid(NameStr(*dbName), false);
! PG_RETURN_INT64(calculate_database_size(dbOid));
}
--- 142,155 ----
{
Name dbName = PG_GETARG_NAME(0);
Oid dbOid = get_database_oid(NameStr(*dbName), false);
+ int64 size;
! size = calculate_database_size(dbOid);
!
! if (!size)
! PG_RETURN_NULL();
!
! PG_RETURN_INT64(size);
}
*************** calculate_tablespace_size(Oid tblspcOid)
*** 184,193 ****
dirdesc = AllocateDir(tblspcPath);
if (!dirdesc)
! ereport(ERROR,
! (errcode_for_file_access(),
! errmsg("could not open tablespace directory \"%s\": %m",
! tblspcPath)));
while ((direntry = ReadDir(dirdesc, tblspcPath)) != NULL)
{
--- 190,196 ----
dirdesc = AllocateDir(tblspcPath);
if (!dirdesc)
! return -1;
while ((direntry = ReadDir(dirdesc, tblspcPath)) != NULL)
{
*************** Datum
*** 226,233 ****
pg_tablespace_size_oid(PG_FUNCTION_ARGS)
{
Oid tblspcOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_tablespace_size(tblspcOid));
}
Datum
--- 229,242 ----
pg_tablespace_size_oid(PG_FUNCTION_ARGS)
{
Oid tblspcOid = PG_GETARG_OID(0);
+ int64 size;
! size = calculate_tablespace_size(tblspcOid);
!
! if (size < 0)
! PG_RETURN_NULL();
!
! PG_RETURN_INT64(size);
}
Datum
*************** pg_tablespace_size_name(PG_FUNCTION_ARGS
*** 235,242 ****
{
Name tblspcName = PG_GETARG_NAME(0);
Oid tblspcOid = get_tablespace_oid(NameStr(*tblspcName), false);
! PG_RETURN_INT64(calculate_tablespace_size(tblspcOid));
}
--- 244,257 ----
{
Name tblspcName = PG_GETARG_NAME(0);
Oid tblspcOid = get_tablespace_oid(NameStr(*tblspcName), false);
+ int64 size;
! size = calculate_tablespace_size(tblspcOid);
!
! if (size < 0)
! PG_RETURN_NULL();
!
! PG_RETURN_INT64(size);
}
*************** pg_relation_size(PG_FUNCTION_ARGS)
*** 289,295 ****
Relation rel;
int64 size;
! rel = relation_open(relOid, AccessShareLock);
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
--- 304,313 ----
Relation rel;
int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
size = calculate_relation_size(&(rel->rd_node), rel->rd_backend,
forkname_to_number(text_to_cstring(forkName)));
*************** calculate_toast_table_size(Oid toastreli
*** 339,352 ****
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
ForkNumber forkNum;
- rel = relation_open(relOid, AccessShareLock);
-
/*
* heap size, including FSM and VM
*/
--- 357,367 ----
* those won't have attached toast tables, but they can have multiple forks.
*/
static int64
! calculate_table_size(Relation rel)
{
int64 size = 0;
ForkNumber forkNum;
/*
* heap size, including FSM and VM
*/
*************** calculate_table_size(Oid relOid)
*** 360,367 ****
if (OidIsValid(rel->rd_rel->reltoastrelid))
size += calculate_toast_table_size(rel->rd_rel->reltoastrelid);
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 375,380 ----
*************** calculate_table_size(Oid relOid)
*** 371,382 ****
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Oid relOid)
{
int64 size = 0;
- Relation rel;
-
- rel = relation_open(relOid, AccessShareLock);
/*
* Aggregate all indexes on the given relation
--- 384,392 ----
* Can be applied safely to an index, but you'll just get zero.
*/
static int64
! calculate_indexes_size(Relation rel)
{
int64 size = 0;
/*
* Aggregate all indexes on the given relation
*************** calculate_indexes_size(Oid relOid)
*** 405,412 ****
list_free(index_oids);
}
- relation_close(rel, AccessShareLock);
-
return size;
}
--- 415,420 ----
*************** Datum
*** 414,429 ****
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_table_size(relOid));
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_indexes_size(relOid));
}
/*
--- 422,459 ----
pg_table_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
!
! size = calculate_table_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
Datum
pg_indexes_size(PG_FUNCTION_ARGS)
{
Oid relOid = PG_GETARG_OID(0);
+ Relation rel;
+ int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
!
! size = calculate_indexes_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
*************** pg_indexes_size(PG_FUNCTION_ARGS)
*** 431,437 ****
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Oid Relid)
{
int64 size;
--- 461,467 ----
* including heap data, index data, toast data, FSM, VM.
*/
static int64
! calculate_total_relation_size(Relation Rel)
{
int64 size;
*************** calculate_total_relation_size(Oid Relid)
*** 439,450 ****
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Relid);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Relid);
return size;
}
--- 469,480 ----
* Aggregate the table size, this includes size of the heap, toast and
* toast index with free space and visibility map
*/
! size = calculate_table_size(Rel);
/*
* Add size of all attached indexes as well
*/
! size += calculate_indexes_size(Rel);
return size;
}
*************** calculate_total_relation_size(Oid Relid)
*** 452,460 ****
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relid = PG_GETARG_OID(0);
! PG_RETURN_INT64(calculate_total_relation_size(relid));
}
/*
--- 482,501 ----
Datum
pg_total_relation_size(PG_FUNCTION_ARGS)
{
! Oid relOid = PG_GETARG_OID(0);
! Relation rel;
! int64 size;
! rel = try_relation_open(relOid, AccessShareLock);
!
! if (rel == NULL)
! PG_RETURN_NULL();
!
! size = calculate_total_relation_size(rel);
!
! relation_close(rel, AccessShareLock);
!
! PG_RETURN_INT64(size);
}
/*
On 23.12.2011 02:01, Phil Sorber wrote:
On Thu, Dec 22, 2011 at 3:19 PM, Robert Haas<robertmhaas@gmail.com> wrote:
On Thu, Dec 22, 2011 at 2:02 PM, Phil Sorber<phil@omniti.com> wrote:
On Thu, Dec 22, 2011 at 1:33 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
Robert Haas<robertmhaas@gmail.com> writes:
I'm wondering if we oughta just return NULL and be done with it.
+1. There are multiple precedents for that sort of response, which we
introduced exactly so that "SELECT some_function(oid) FROM some_catalog"
wouldn't fail just because one of the rows had gotten deleted by the
time the scan got to it. I don't think it's necessary for the
relation-size functions to be any smarter. Indeed, I'd assumed that's
all that Phil's patch did, since I'd not looked closer till just now.Here it is without the checking for recently dead. If it can't open
the relation it simply returns NULL.I think we probably ought to make pg_database_size() and
pg_tablespace_size() behave similarly.Changes added.
Looks good to me, committed. I added a sentence to the docs mentioning
the new behavior, and also a code comment to explain why returning NULL
is better than throwing an error.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com