Postgres Pro
Downloads
Home   >   mailing lists

Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument - Mailing list pgsql-hackers

From Rushabh Lathia
Subject Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument
Date
Msg-id CAGPqQf3Yk4tuxZOxGGF2rdrZZFDXVCVeMTA6m+iwzvE5evm6qg@mail.gmail.com
Whole thread Raw
In response to Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument  (Satoshi Nagayasu <snaga@uptime.jp>)
List pgsql-hackers
Tree view
Hi Satoshi,

I spent some time on the revised version on the patch(pgstattuple_regclass_v2.diff)
and here are my comments.

.) Patch get applies cleanly on PG master branch
.) Successful build and database creation
.) Basic test coverage included in the patch
.) make check running cleanly

Basically goal of the patch is to allow specifying a relation/index with
several expressions, 'relname', 'schemaname.relname' and oid in all pgstattuple
functions. To achieve the same patch introduced another version of pgstattuple
functions which takes regclass as input args. To make it backward compatible
we kept the pgstatetuple functions with TEXT input arg.

In the mail thread we decided that pgstattuple(text) will be depreciated in
the future release and we need to document that. Which is missing in the patch.

Apart from that few comments in the C code to explain "why multiple version
of the pgstattuple function ?" would be really helpful for future understanding
purpose.

Thanks,



On Tue, Jul 16, 2013 at 11:42 AM, Satoshi Nagayasu <snaga@uptime.jp> wrote:
Hi Rushabh,


(2013/07/16 14:58), Rushabh Lathia wrote:
Hello Satoshi,

I assigned myself for the reviewer of this patch. Issue status is waiting on
author.

Thank you for picking it up.


Now looking at the discussion under the thread it seems like we are waiting
for the suggestion for the new function name, right ?

Yes.


I am wondering why actually we need new name ? Can't we just overload the
same function and provide two version of the functions ?

I think the major reason is to avoid some confusion with old and new
function arguments.

My thought here is that having both arguments (text and regclass)
for each function is a good choice to clean up interfaces with keeping
the backward-compatibility.


In the last thread Fujii just did the same for pg_relpages and it seems
like an
good to go approach, isn't it ? Am I missing anything here ?

I just posted a revised patch to handle the issue in three functions
of the pgstattuple module. Please take a look.


Regards,
--
Satoshi Nagayasu <snaga@uptime.jp>
Uptime Technologies, LLC. http://www.uptime.jp



--
Rushabh Lathia

pgsql-hackers by date:

Previous
From: Tatsuo Ishii
Date:
Subject: Re: [PATCH] pgbench --throttle (submission 7 - with lag measurement)
Next
From: Soroosh Sardari
Date:
Subject: A general Q about index