(2013/07/18 2:31), Fujii Masao wrote:
On Tue, Jul 16, 2013 at 3:00 PM, Satoshi Nagayasu <sn...@uptime.jp> wrote:
(2013/07/04 3:58), Fujii Masao wrote:
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.

Yes, please.

Updated docs and code comments, etc. PFA.

Any comments?

'make installcheck' failed in my machine.

Hmm, it works on my box...

Do we need to remove pgstattuple--1.1.sql and create pgstattuple--1.1--1.2.sql?

+/* contrib/pgstattuple/pgstattuple--1.1.sql */

Typo: s/1.1/1.2

Done.

You seem to have forgotten to update pgstattuple.c.

Should I change something in pgstattuple.c?

I just changed CREATE FUNCTION statement for pgstattuple
to replace oid input arg with the regclass.

Regards,
--
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..41e90e3 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -39,12 +39,24 @@
 #include "utils/rel.h"
 
 
+/*
+ * Because of backward-compatibility issue, we have decided to have
+ * two types of interfaces, with regclass-type input arg and text-type
+ * input arg, for each function.
+ *
+ * Those functions which have text-type input arg will be depreciated
+ * in the future release.
+ */
 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 +109,8 @@ typedef struct GinIndexStat
        int64           pending_tuples;
 } GinIndexStat;
 
+static Datum pgstatindex_impl(Relation, FunctionCallInfo);
+
 /* ------------------------------------------------------
  * pgstatindex()
  *
@@ -109,11 +123,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 +132,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 +311,7 @@ pgstatindex(PG_FUNCTION_ARGS)
                result = HeapTupleGetDatum(tuple);
        }
 
-       PG_RETURN_DATUM(result);
+       return result;
 }
 
 /* --------------------------------------------------------
@@ -311,6 +348,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.1--1.2.sql 
b/contrib/pgstattuple/pgstattuple--1.1--1.2.sql
new file mode 100644
index 0000000..3c9c2a2
--- /dev/null
+++ b/contrib/pgstattuple/pgstattuple--1.1--1.2.sql
@@ -0,0 +1,39 @@
+/* contrib/pgstattuple/pgstattuple--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pgstattuple UPDATE TO '1.2'" to load this file. 
\quit
+
+DROP FUNCTION pgstattuple(oid);
+
+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 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 regclass)
+RETURNS BIGINT
+AS 'MODULE_PATHNAME', 'pg_relpagesbyid'
+LANGUAGE C STRICT;
+
diff --git a/contrib/pgstattuple/pgstattuple--1.1.sql 
b/contrib/pgstattuple/pgstattuple--1.1.sql
deleted file mode 100644
index b21fbf8..0000000
--- a/contrib/pgstattuple/pgstattuple--1.1.sql
+++ /dev/null
@@ -1,58 +0,0 @@
-/* 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 oid,
-    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 pg_relpages(IN relname text)
-RETURNS BIGINT
-AS 'MODULE_PATHNAME', 'pg_relpages'
-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--1.2.sql 
b/contrib/pgstattuple/pgstattuple--1.2.sql
new file mode 100644
index 0000000..7477d0c
--- /dev/null
+++ b/contrib/pgstattuple/pgstattuple--1.2.sql
@@ -0,0 +1,77 @@
+/* contrib/pgstattuple/pgstattuple--1.2.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);
 
diff --git a/doc/src/sgml/pgstattuple.sgml b/doc/src/sgml/pgstattuple.sgml
index f2bc2a6..64448f5 100644
--- a/doc/src/sgml/pgstattuple.sgml
+++ b/doc/src/sgml/pgstattuple.sgml
@@ -22,7 +22,7 @@
    </indexterm>
 
     <term>
-     <function>pgstattuple(text) returns record</>
+     <function>pgstattuple(regclass) returns record</>
     </term>
 
     <listitem>
@@ -30,7 +30,7 @@
       <function>pgstattuple</function> returns a relation's physical length,
       percentage of <quote>dead</> tuples, and other info. This may help users
       to determine whether vacuum is necessary or not.  The argument is the
-      target relation's name (optionally schema-qualified).
+      target relation's name (optionally schema-qualified) or OID.
       For example:
 <programlisting>
 test=> SELECT * FROM pgstattuple('pg_catalog.pg_proc');
@@ -125,13 +125,15 @@ free_percent       | 1.95
 
    <varlistentry>
     <term>
-     <function>pgstattuple(oid) returns record</>
+     <function>pgstattuple(text) returns record</>
     </term>
 
     <listitem>
      <para>
-      This is the same as <function>pgstattuple(text)</function>, except
-      that the target relation is specified by OID.
+      This is the same as <function>pgstattuple(regclass)</function>, except
+      that the target relation is specified by TEXT. This function is kept
+      because of backward-compatibility so far, and will be depreciated in
+      the future release.
      </para>
     </listitem>
    </varlistentry>
@@ -141,7 +143,7 @@ free_percent       | 1.95
     <indexterm>
      <primary>pgstatindex</primary>
     </indexterm>
-     <function>pgstatindex(text) returns record</>
+     <function>pgstatindex(regclass) returns record</>
     </term>
 
     <listitem>
@@ -253,6 +255,21 @@ leaf_fragmentation | 0
 
    <varlistentry>
     <term>
+     <function>pgstatindex(text) returns record</>
+    </term>
+
+    <listitem>
+     <para>
+      This is the same as <function>pgstatindex(regclass)</function>, except
+      that the target index is specified by TEXT. This function is kept
+      because of backward-compatibility so far, and will be depreciated in
+      the future release.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
+    <term>
      <indexterm>
       <primary>pgstatginindex</primary>
      </indexterm>
@@ -316,7 +333,7 @@ pending_tuples | 0
      <indexterm>
       <primary>pg_relpages</primary>
      </indexterm>
-     <function>pg_relpages(text) returns bigint</>
+     <function>pg_relpages(regclass) returns bigint</>
     </term>
 
     <listitem>
@@ -326,6 +343,22 @@ pending_tuples | 0
      </para>
     </listitem>
    </varlistentry>
+
+   <varlistentry>
+    <term>
+     <function>pg_relpages(text) returns bigint</>
+    </term>
+
+    <listitem>
+     <para>
+      This is the same as <function>pg_relpages(regclass)</function>, except
+      that the target relation is specified by TEXT. This function is kept
+      because of backward-compatibility so far, and will be depreciated in
+      the future release.
+     </para>
+    </listitem>
+   </varlistentry>
+
   </variablelist>
  </sect2>
 
-- 
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