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 CAGPqQf0nC9t7VVKj0_LCyJ0pM1hq_jN-c9qT9O94=3Q0K6iOnQ@mail.gmail.com
Whole thread Raw
In response to Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument
List pgsql-hackers
Hello Satoshi,

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

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

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

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 ?

Regards,




On Thu, Jul 4, 2013 at 12:28 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Wed, Jun 26, 2013 at 12:39 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Jun 20, 2013 at 2:32 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> On Thu, Jun 20, 2013 at 11:43 AM, Satoshi Nagayasu <snaga@uptime.jp> wrote:
>>> (2013/06/17 4:02), Fujii Masao wrote:
>>>>
>>>> On Sat, Mar 9, 2013 at 3:23 PM, Satoshi Nagayasu <snaga@uptime.jp> wrote:
>>>>>
>>>>> It is obviously easy to keep two types of function interfaces,
>>>>> one with regclass-type and another with text-type, in the next
>>>>> release for backward-compatibility like below:
>>>>>
>>>>> pgstattuple(regclass)  -- safer interface.
>>>>> pgstattuple(text)      -- will be depreciated in the future release.
>>>>
>>>>
>>>> So you're thinking to remove pgstattuple(oid) soon?
>>>
>>>
>>> AFAIK, a regclass type argument would accept an OID value,
>>> which means regclass type has upper-compatibility against
>>> oid type.
>>>
>>> So, even if the declaration is changed, compatibility could
>>> be kept actually. This test case (in sql/pgstattuple.sql)
>>> confirms that.
>>>
>>> ----------------------------------------------------------------
>>> select * from pgstatindex('myschema.test_pkey'::regclass::oid);
>>>  version | tree_level | index_size | root_block_no | internal_pages |
>>> leaf_pages | empty_pages | deleted_pages | avg_leaf_density |
>>> leaf_fragmentation
>>> ---------+------------+------------+---------------+----------------+------------+-------------+---------------+------------------+--------------------
>>>        2 |          0 |       8192 |             1 |              0 |
>>> 1 |           0 |             0 |             0.79 |        0
>>> (1 row)
>>> ----------------------------------------------------------------
>>>
>>>
>>>>> Having both interfaces for a while would allow users to have enough
>>>>> time to rewrite their applications.
>>>>>
>>>>> Then, we will be able to obsolete (or just drop) old interfaces
>>>>> in the future release, maybe 9.4 or 9.5. I think this approach
>>>>> would minimize an impact of such interface change.
>>>>>
>>>>> So, I think we can clean up function arguments in the pgstattuple
>>>>> module, and also we can have two interfaces, both regclass and text,
>>>>> for the next release.
>>>>>
>>>>> Any comments?
>>>>
>>>>
>>>> In the document, you should mark old functions as deprecated.
>>>
>>>
>>> I'm still considering changing the function name as Tom pointed
>>> out. How about "pgstatbtindex"?
>>
>> Or just adding pgstatindex(regclass)?
>>
>>> In fact, pgstatindex does support only BTree index.
>>> So, "pgstatbtindex" seems to be more appropriate for this function.
>>
>> Can most ordinary users realize "bt" means "btree"?
>>
>>> We can keep having both (old) pgstatindex and (new) pgstatbtindex
>>> during next 2-3 major releases, and the old one will be deprecated
>>> after that.
>>
>> Since pg_relpages(oid) doesn't exist, pg_relpages() is in the same
>> situation as pgstatindex(), i.e., we cannot just replace pg_relpages(text)
>> with pg_relpages(regclass) for the backward-compatibility. How do you
>> think we should solve the pg_relpages() problem? Rename? Just
>> add pg_relpages(regclass)?
>
> Adding a function with a new name seems likely to be smoother, since
> that way you don't have to worry about problems with function calls
> being thought ambiguous.

Could you let me know the example that this problem happens?

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

Regards,

--
Fujii Masao


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers




--
Rushabh Lathia

pgsql-hackers by date:

Previous
From: Amit kapila
Date:
Subject: Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Next
From: Satoshi Nagayasu
Date:
Subject: Re: Fix pgstattuple/pgstatindex to use regclass-type as the argument