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

From: Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
To: Satoshi Nagayasu <snaga(at)uptime(dot)jp>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument
Date: 2013-07-18 04:49:33
Message-ID: CAGPqQf1R-Hc2hvEN_B4xreFrv+F5bso6G6=AEvs6eAm9scvqjg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 18, 2013 at 9:40 AM, Satoshi Nagayasu <snaga(at)uptime(dot)jp> wrote:

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

Looks good.

>
>
> Any comments?
>>>
>>
>> 'make installcheck' failed in my machine.
>>
>
> Hmm, it works on my box...

Works for me too.

Overall looks good to me.

>
>
> 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 <snaga(at)uptime(dot)jp>
> Uptime Technologies, LLC. http://www.uptime.jp
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

Thanks,
Rushabh Lathia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2013-07-18 04:51:36 Re: [COMMITTERS] pgsql: Add support for REFRESH MATERIALIZED VIEW CONCURRENTLY.
Previous Message Tatsuo Ishii 2013-07-18 04:42:34 Re: [PATCH] pgbench --throttle (submission 7 - with lag measurement)