Hello, 

> Agreed. Attached patch introduces the pgstatginindex() which now reports
> GIN version number, number of pages in the pending list and number of
> tuples in the pending list, as information about a GIN index.

It seems fine on the whole, and I have some suggestions.

1. This patch applies current git head cleanly, but installation
  ends up with failure because of the lack of
  pgstattuple--1.0--1.1.sql which added in Makefile.

2. I feel somewhat uneasy with size for palloc's (it's too long),
  and BuildTupleFromCString used instead of heap_from_tuple.. But
  it would lead additional modification for existent simillars.

  You can leave that if you prefer to keep this patch smaller,
  but it looks to me more preferable to construct the result
  tuple not via c-strings in some aspects. (*1)

3. pgstatginidex shows only version, pending_pages, and
  pendingtuples. Why don't you show the other properties such as
  entry pages, data pages, entries, and total pages as
  pgstatindex does?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


(*1) Sample

diff --git a/contrib/pgstattuple/pgstatindex.c 
b/contrib/pgstattuple/pgstatindex.c
index 8a2ae85..71c2023 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -29,2 +29,3 @@
 
+#include "access/htup_details.h"
 #include "access/gin_private.h"
@@ -39,3 +40,2 @@
 
-
 extern Datum pgstatindex(PG_FUNCTION_ARGS);
@@ -330,4 +330,5 @@ pgstatginindex(PG_FUNCTION_ARGS)
        int                     j;
-       char       *values[3];
+       Datum       values[3];
        Datum           result;
+       bool nulls[3] = {false, false, false};
 
@@ -376,11 +377,6 @@ pgstatginindex(PG_FUNCTION_ARGS)
        j = 0;
-       values[j] = palloc(32);
-       snprintf(values[j++], 32, "%d", stats.version);
-       values[j] = palloc(32);
-       snprintf(values[j++], 32, "%u", stats.pending_pages);
-       values[j] = palloc(64);
-       snprintf(values[j++], 64, INT64_FORMAT, stats.pending_tuples);
-
-       tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
-                                                                  values);
+       values[j++] = Int32GetDatum(stats.version);
+       values[j++] = UInt32GetDatum(stats.pending_pages);
+       values[j++] = Int64GetDatumFast(stats.pending_tuples);
+       tuple = heap_form_tuple(tupleDesc, values, nulls);



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