Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument

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(at)uptime(dot)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(at)uptime(dot)jp>
> Uptime Technologies, LLC. http://www.uptime.jp
>

--
Rushabh Lathia