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

On Thu, Jun 20, 2013 at 2:32 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Thu, Jun 20, 2013 at 11:43 AM, Satoshi Nagayasu <snaga(at)uptime(dot)jp> wrote:
>> (2013/06/17 4:02), Fujii Masao wrote:
>>>
>>> On Sat, Mar 9, 2013 at 3:23 PM, Satoshi Nagayasu <snaga(at)uptime(dot)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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company