(2013/07/04 3:58), Fujii Masao wrote:
> On Wed, Jun 26, 2013 at 12:39 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> On Thu, Jun 20, 2013 at 2:32 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
>>> Since pg_relpages(oid) doesn't exist, pg_relpages() is in the same
>>> situation as pgstatindex(), i.e., we cannot just replace pg_relpages(text)
>>> with pg_relpages(regclass) for the backward-compatibility. How do you
>>> think we should solve the pg_relpages() problem? Rename? Just
>>> add pg_relpages(regclass)?
>>
>> Adding a function with a new name seems likely to be smoother, since
>> that way you don't have to worry about problems with function calls
>> being thought ambiguous.
> 
> Could you let me know the example that this problem happens?
> 
> For the test, I just implemented the regclass-version of pg_relpages()
> (patch attached) and tested some cases. But I could not get that problem.
> 
>      SELECT pg_relpages('hoge');    -- OK
>      SELECT pg_relpages(oid) FROM pg_class WHERE relname = 'hoge';    -- OK
>      SELECT pg_relpages(relname) FROM pg_class WHERE relname = 'hoge';    -- 
> OK

In the attached patch, I cleaned up three functions to have
two types of arguments for each, text and regclass.

  pgstattuple(text)
  pgstattuple(regclass)
  pgstatindex(text)
  pgstatindex(regclass)
  pg_relpages(text)
  pg_relpages(regclass)

I still think a regclass argument is more appropriate for passing
relation/index name to a function than text-type, but having both
arguments in each function seems to be a good choice at this moment,
in terms of backward-compatibility.

Docs needs to be updated if this change going to be applied.

Any comments?
-- 
Satoshi Nagayasu <sn...@uptime.jp>
Uptime Technologies, LLC. http://www.uptime.jp
diff --git a/contrib/pgstattuple/Makefile b/contrib/pgstattuple/Makefile
index fc893d8..957742a 100644
--- a/contrib/pgstattuple/Makefile
+++ b/contrib/pgstattuple/Makefile
@@ -4,7 +4,7 @@ MODULE_big      = pgstattuple
 OBJS           = pgstattuple.o pgstatindex.o
 
 EXTENSION = pgstattuple
-DATA = pgstattuple--1.1.sql pgstattuple--1.0--1.1.sql 
pgstattuple--unpackaged--1.0.sql
+DATA = pgstattuple--1.2.sql pgstattuple--1.0--1.1.sql 
pgstattuple--unpackaged--1.0.sql
 
 REGRESS = pgstattuple
 
diff --git a/contrib/pgstattuple/expected/pgstattuple.out 
b/contrib/pgstattuple/expected/pgstattuple.out
index ab28f50..eaba306 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -11,12 +11,24 @@ select * from pgstattuple('test'::text);
          0 |           0 |         0 |             0 |                0 |      
        0 |                  0 |          0 |            0
 (1 row)
 
+select * from pgstattuple('test'::name);
+ table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | 
dead_tuple_len | dead_tuple_percent | free_space | free_percent 
+-----------+-------------+-----------+---------------+------------------+----------------+--------------------+------------+--------------
+         0 |           0 |         0 |             0 |                0 |      
        0 |                  0 |          0 |            0
+(1 row)
+
 select * from pgstattuple('test'::regclass);
  table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | 
dead_tuple_len | dead_tuple_percent | free_space | free_percent 
 
-----------+-------------+-----------+---------------+------------------+----------------+--------------------+------------+--------------
          0 |           0 |         0 |             0 |                0 |      
        0 |                  0 |          0 |            0
 (1 row)
 
+select * from pgstattuple('test'::regclass::oid);
+ table_len | tuple_count | tuple_len | tuple_percent | dead_tuple_count | 
dead_tuple_len | dead_tuple_percent | free_space | free_percent 
+-----------+-------------+-----------+---------------+------------------+----------------+--------------------+------------+--------------
+         0 |           0 |         0 |             0 |                0 |      
        0 |                  0 |          0 |            0
+(1 row)
+
 select * from pgstatindex('test_pkey');
  version | tree_level | index_size | root_block_no | internal_pages | 
leaf_pages | empty_pages | deleted_pages | avg_leaf_density | 
leaf_fragmentation 
 
---------+------------+------------+---------------+----------------+------------+-------------+---------------+------------------+--------------------
diff --git a/contrib/pgstattuple/pgstatindex.c 
b/contrib/pgstattuple/pgstatindex.c
index 97f897e..9ec74e7 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -40,11 +40,15 @@
 
 
 extern Datum pgstatindex(PG_FUNCTION_ARGS);
+extern Datum pgstatindexbyid(PG_FUNCTION_ARGS);
 extern Datum pg_relpages(PG_FUNCTION_ARGS);
+extern Datum pg_relpagesbyid(PG_FUNCTION_ARGS);
 extern Datum pgstatginindex(PG_FUNCTION_ARGS);
 
 PG_FUNCTION_INFO_V1(pgstatindex);
+PG_FUNCTION_INFO_V1(pgstatindexbyid);
 PG_FUNCTION_INFO_V1(pg_relpages);
+PG_FUNCTION_INFO_V1(pg_relpagesbyid);
 PG_FUNCTION_INFO_V1(pgstatginindex);
 
 #define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
@@ -97,6 +101,8 @@ typedef struct GinIndexStat
        int64           pending_tuples;
 } GinIndexStat;
 
+static Datum pgstatindex_impl(Relation, FunctionCallInfo);
+
 /* ------------------------------------------------------
  * pgstatindex()
  *
@@ -109,11 +115,6 @@ pgstatindex(PG_FUNCTION_ARGS)
        text       *relname = PG_GETARG_TEXT_P(0);
        Relation        rel;
        RangeVar   *relrv;
-       Datum           result;
-       BlockNumber nblocks;
-       BlockNumber blkno;
-       BTIndexStat indexStat;
-       BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
 
        if (!superuser())
                ereport(ERROR,
@@ -123,6 +124,34 @@ pgstatindex(PG_FUNCTION_ARGS)
        relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
        rel = relation_openrv(relrv, AccessShareLock);
 
+       PG_RETURN_DATUM( pgstatindex_impl(rel, fcinfo) );
+}
+
+Datum
+pgstatindexbyid(PG_FUNCTION_ARGS)
+{
+       Oid                     relid = PG_GETARG_OID(0);
+       Relation        rel;
+
+       if (!superuser())
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                (errmsg("must be superuser to use pgstattuple 
functions"))));
+
+       rel = relation_open(relid, AccessShareLock);
+
+       PG_RETURN_DATUM( pgstatindex_impl(rel, fcinfo) );
+}
+
+static Datum
+pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo)
+{
+       Datum           result;
+       BlockNumber nblocks;
+       BlockNumber blkno;
+       BTIndexStat indexStat;
+       BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
+
        if (!IS_INDEX(rel) || !IS_BTREE(rel))
                elog(ERROR, "relation \"%s\" is not a btree index",
                         RelationGetRelationName(rel));
@@ -274,7 +303,7 @@ pgstatindex(PG_FUNCTION_ARGS)
                result = HeapTupleGetDatum(tuple);
        }
 
-       PG_RETURN_DATUM(result);
+       return result;
 }
 
 /* --------------------------------------------------------
@@ -311,6 +340,29 @@ pg_relpages(PG_FUNCTION_ARGS)
        PG_RETURN_INT64(relpages);
 }
 
+Datum
+pg_relpagesbyid(PG_FUNCTION_ARGS)
+{
+       Oid                     relid = PG_GETARG_OID(0);
+       int64           relpages;
+       Relation        rel;
+
+       if (!superuser())
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                (errmsg("must be superuser to use pgstattuple 
functions"))));
+
+       rel = relation_open(relid, AccessShareLock);
+
+       /* note: this will work OK on non-local temp tables */
+
+       relpages = RelationGetNumberOfBlocks(rel);
+
+       relation_close(rel, AccessShareLock);
+
+       PG_RETURN_INT64(relpages);
+}
+
 /* ------------------------------------------------------
  * pgstatginindex()
  *
diff --git a/contrib/pgstattuple/pgstattuple--1.2.sql 
b/contrib/pgstattuple/pgstattuple--1.2.sql
new file mode 100644
index 0000000..a442e6a
--- /dev/null
+++ b/contrib/pgstattuple/pgstattuple--1.2.sql
@@ -0,0 +1,77 @@
+/* contrib/pgstattuple/pgstattuple--1.1.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION pgstattuple" to load this file. \quit
+
+CREATE FUNCTION pgstattuple(IN relname text,
+    OUT table_len BIGINT,              -- physical table length in bytes
+    OUT tuple_count BIGINT,            -- number of live tuples
+    OUT tuple_len BIGINT,              -- total tuples length in bytes
+    OUT tuple_percent FLOAT8,          -- live tuples in %
+    OUT dead_tuple_count BIGINT,       -- number of dead tuples
+    OUT dead_tuple_len BIGINT,         -- total dead tuples length in bytes
+    OUT dead_tuple_percent FLOAT8,     -- dead tuples in %
+    OUT free_space BIGINT,             -- free space in bytes
+    OUT free_percent FLOAT8)           -- free space in %
+AS 'MODULE_PATHNAME', 'pgstattuple'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION pgstattuple(IN reloid regclass,
+    OUT table_len BIGINT,              -- physical table length in bytes
+    OUT tuple_count BIGINT,            -- number of live tuples
+    OUT tuple_len BIGINT,              -- total tuples length in bytes
+    OUT tuple_percent FLOAT8,          -- live tuples in %
+    OUT dead_tuple_count BIGINT,       -- number of dead tuples
+    OUT dead_tuple_len BIGINT,         -- total dead tuples length in bytes
+    OUT dead_tuple_percent FLOAT8,     -- dead tuples in %
+    OUT free_space BIGINT,             -- free space in bytes
+    OUT free_percent FLOAT8)           -- free space in %
+AS 'MODULE_PATHNAME', 'pgstattuplebyid'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION pgstatindex(IN relname text,
+    OUT version INT,
+    OUT tree_level INT,
+    OUT index_size BIGINT,
+    OUT root_block_no BIGINT,
+    OUT internal_pages BIGINT,
+    OUT leaf_pages BIGINT,
+    OUT empty_pages BIGINT,
+    OUT deleted_pages BIGINT,
+    OUT avg_leaf_density FLOAT8,
+    OUT leaf_fragmentation FLOAT8)
+AS 'MODULE_PATHNAME', 'pgstatindex'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION pgstatindex(IN relname regclass,
+    OUT version INT,
+    OUT tree_level INT,
+    OUT index_size BIGINT,
+    OUT root_block_no BIGINT,
+    OUT internal_pages BIGINT,
+    OUT leaf_pages BIGINT,
+    OUT empty_pages BIGINT,
+    OUT deleted_pages BIGINT,
+    OUT avg_leaf_density FLOAT8,
+    OUT leaf_fragmentation FLOAT8)
+AS 'MODULE_PATHNAME', 'pgstatindexbyid'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION pg_relpages(IN relname text)
+RETURNS BIGINT
+AS 'MODULE_PATHNAME', 'pg_relpages'
+LANGUAGE C STRICT;
+
+CREATE FUNCTION pg_relpages(IN relname regclass)
+RETURNS BIGINT
+AS 'MODULE_PATHNAME', 'pg_relpagesbyid'
+LANGUAGE C STRICT;
+
+/* New stuff in 1.1 begins here */
+
+CREATE FUNCTION pgstatginindex(IN relname regclass,
+    OUT version INT4,
+    OUT pending_pages INT4,
+    OUT pending_tuples BIGINT)
+AS 'MODULE_PATHNAME', 'pgstatginindex'
+LANGUAGE C STRICT;
diff --git a/contrib/pgstattuple/pgstattuple.control 
b/contrib/pgstattuple/pgstattuple.control
index fcfd36f..a7cf47f 100644
--- a/contrib/pgstattuple/pgstattuple.control
+++ b/contrib/pgstattuple/pgstattuple.control
@@ -1,5 +1,5 @@
 # pgstattuple extension
 comment = 'show tuple-level statistics'
-default_version = '1.1'
+default_version = '1.2'
 module_pathname = '$libdir/pgstattuple'
 relocatable = true
diff --git a/contrib/pgstattuple/sql/pgstattuple.sql 
b/contrib/pgstattuple/sql/pgstattuple.sql
index 8cb350d..76369c5 100644
--- a/contrib/pgstattuple/sql/pgstattuple.sql
+++ b/contrib/pgstattuple/sql/pgstattuple.sql
@@ -9,12 +9,23 @@ CREATE EXTENSION pgstattuple;
 create table test (a int primary key, b int[]);
 
 select * from pgstattuple('test'::text);
+select * from pgstattuple('test'::name);
 select * from pgstattuple('test'::regclass);
+select * from pgstattuple('test'::regclass::oid);
+select pgstattuple(relname) from pg_class where relname = 'test' and relkind = 
'r';
 
-select * from pgstatindex('test_pkey');
+select * from pgstatindex('test_pkey'::text);
+select * from pgstatindex('test_pkey'::name);
+select * from pgstatindex('test_pkey'::regclass);
+select * from pgstatindex('test_pkey'::regclass::oid);
+select pgstatindex(relname) from pg_class where relname = 'test_pkey' and 
relkind = 'i';
 
 select pg_relpages('test');
 select pg_relpages('test_pkey');
+select pg_relpages('test_pkey'::name);
+select pg_relpages('test_pkey'::regclass);
+select pg_relpages('test_pkey'::regclass::oid);
+select pg_relpages(relname) from pg_class where relname = 'test_pkey' and 
relkind = 'i';
 
 create index test_ginidx on test using gin (b);
 
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to