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

On Wed, Jun 26, 2013 at 12:39 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> 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.

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