Re: Fix pgstatindex using for large indexes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tatsuhito Kasahara <kasahara(dot)tatsuhito(at)oss(dot)ntt(dot)co(dot)jp>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Fix pgstatindex using for large indexes
Date: 2008-03-21 02:45:36
Message-ID: 11677.1206067536@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Tatsuhito Kasahara <kasahara(dot)tatsuhito(at)oss(dot)ntt(dot)co(dot)jp> writes:
> Tom Lane wrote:
>> Most places where we've dealt with this before, we use double, which is
>> guaranteed to be available whereas uint64 is not ...

> Oh I see.

> I fix the patch.
> # I changed "max_avail" and "free_space" to double.

I took a closer look at this, and noticed that you were still redefining
the output parameters of the function as BIGINT:

***************
*** 33,48 ****
-- pgstatindex
--
CREATE OR REPLACE FUNCTION pgstatindex(IN relname text,
! OUT version int4,
! OUT tree_level int4,
! OUT index_size int4,
! OUT root_block_no int4,
! OUT internal_pages int4,
! OUT leaf_pages int4,
! OUT empty_pages int4,
! OUT deleted_pages int4,
! OUT avg_leaf_density float8,
! OUT leaf_fragmentation float8)
AS 'MODULE_PATHNAME', 'pgstatindex'
LANGUAGE C STRICT;

--- 33,48 ----
-- pgstatindex
--
CREATE OR REPLACE FUNCTION pgstatindex(IN relname text,
! OUT version INT,
! OUT tree_level INT,
! OUT index_size BIGINT,
! OUT root_block_no INT,
! OUT internal_pages BIGINT,
! OUT leaf_pages BIGINT,
! OUT empty_pages BIGINT,
! OUT deleted_pages BIGINT,
! OUT avg_leaf_density FLOAT8,
! OUT leaf_fragmentation FLOAT8)
AS 'MODULE_PATHNAME', 'pgstatindex'
LANGUAGE C STRICT;

This is inconsistent --- if int64 isn't actually available, it's not
likely to work very well. It seems to me that we should either change
the output parameters to float8, or stick with the original version of
the patch and just accept that it will give overflowed answers on
machines without working int64.

Given that there is no problem until you get into multi-terabyte
indexes, which are hardly likely to be getting pushed around on ancient
systems, it's hard to argue that there's really any case against using
bigint. Also I now see that pgstattuple() is using bigint for numbers
that are really much more likely to overflow a broken int64 than
pgstatindex() is, so the argument for float would require us to change
both functions.

In short, I'm willing to drop my opposition to the original form
of the patch.

regards, tom lane

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Brendan Jurd 2008-03-21 03:02:39 Re: [PATCHES] [GENERAL] Empty arrays with ARRAY[]
Previous Message Alvaro Herrera 2008-03-21 02:41:10 Re: Fix pgstatindex using for large indexes