Обсуждение: Fix pgstatindex using for large indexes

Поиск
Список
Период
Сортировка

Fix pgstatindex using for large indexes

От
Tatsuhito Kasahara
Дата:
Hi.

In pgstatindex.c and pgstattuple.sql, some variables are defined with
int type. So when we try to get informations about a large index by using
pgstatindex, we get strange value of size and density.
Because the values exceed int-max.
# Like following output. I used pgstatindex just after data load.
  So "density" is should be nearly 90.

test=# SELECT * FROM pgstatindex('large_index');
-[ RECORD 1 ]------+------------
version            | 2
tree_level         | 4
index_size         | -1349410816 ★
root_block_no      | 119666
internal_pages     | 28936
leaf_pages         | 1379204
empty_pages        | 0
deleted_pages      | 0
avg_leaf_density   | 60.33 ★
leaf_fragmentation | 0


I think that max_avail and free_space should be uint64.
And the output format for index_size should be "%lld" (INT64_FORMAT).

I made the patch and tryed it. (And it seemed OK.)

test=# SELECT * FROM pgstatindex('large_index');
-[ RECORD 1 ]------+------------
version            | 2
tree_level         | 4
index_size         | 11535491072
root_block_no      | 119666
internal_pages     | 28936
leaf_pages         | 1379204
empty_pages        | 0
deleted_pages      | 0
avg_leaf_density   | 90.64
leaf_fragmentation | 0

I also fix *_pages variables just in case.
Please confirm this.

Best regards.

--
NTT OSS Center
Tatsuhito Kasahara

kasahara.tatsuhito _at_ oss.ntt.co.jp
diff -crN postgresql-8.3.0.org/contrib/pgstattuple/pgstatindex.c postgresql-8.3.0/contrib/pgstattuple/pgstatindex.c
*** postgresql-8.3.0.org/contrib/pgstattuple/pgstatindex.c    2007-11-16 06:14:31.000000000 +0900
--- postgresql-8.3.0/contrib/pgstattuple/pgstatindex.c    2008-02-21 22:34:40.000000000 +0900
***************
*** 63,77 ****
      uint32        level;

      uint32        root_pages;
!     uint32        internal_pages;
!     uint32        leaf_pages;
!     uint32        empty_pages;
!     uint32        deleted_pages;

!     uint32        max_avail;
!     uint32        free_space;

!     uint32        fragments;
  }    BTIndexStat;

  /* ------------------------------------------------------
--- 63,77 ----
      uint32        level;

      uint32        root_pages;
!     uint64        internal_pages;
!     uint64        leaf_pages;
!     uint64        empty_pages;
!     uint64        deleted_pages;

!     uint64        max_avail;
!     uint64        free_space;

!     uint64        fragments;
  }    BTIndexStat;

  /* ------------------------------------------------------
***************
*** 87,94 ****
      Relation    rel;
      RangeVar   *relrv;
      Datum        result;
!     uint32        nblocks;
!     uint32        blkno;
      BTIndexStat indexStat;

      if (!superuser())
--- 87,94 ----
      Relation    rel;
      RangeVar   *relrv;
      Datum        result;
!     BlockNumber    nblocks;
!     BlockNumber    blkno;
      BTIndexStat indexStat;

      if (!superuser())
***************
*** 207,213 ****
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.level);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", (indexStat.root_pages +
                                           indexStat.leaf_pages +
                                           indexStat.internal_pages +
                                           indexStat.deleted_pages +
--- 207,213 ----
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.level);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, INT64_FORMAT, (indexStat.root_pages +
                                           indexStat.leaf_pages +
                                           indexStat.internal_pages +
                                           indexStat.deleted_pages +
***************
*** 215,231 ****
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.root_blkno);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.internal_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.leaf_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.empty_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.deleted_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", 100.0 - (float) indexStat.free_space / (float) indexStat.max_avail *
100.0);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", (float) indexStat.fragments / (float) indexStat.leaf_pages * 100.0);

          tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
                                         values);
--- 215,231 ----
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.root_blkno);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, INT64_FORMAT, indexStat.internal_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, INT64_FORMAT, indexStat.leaf_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, INT64_FORMAT, indexStat.empty_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, INT64_FORMAT, indexStat.deleted_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", 100.0 - (double) indexStat.free_space / (double) indexStat.max_avail *
100.0);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", (double) indexStat.fragments / (double) indexStat.leaf_pages * 100.0);

          tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
                                         values);
diff -crN postgresql-8.3.0.org/contrib/pgstattuple/pgstattuple.sql.in
postgresql-8.3.0/contrib/pgstattuple/pgstattuple.sql.in
*** postgresql-8.3.0.org/contrib/pgstattuple/pgstattuple.sql.in    2007-11-13 13:24:28.000000000 +0900
--- postgresql-8.3.0/contrib/pgstattuple/pgstattuple.sql.in    2008-02-21 21:33:02.000000000 +0900
***************
*** 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;


Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
Tatsuhito Kasahara <kasahara.tatsuhito@oss.ntt.co.jp> writes:
> In pgstatindex.c and pgstattuple.sql, some variables are defined with
> int type. So when we try to get informations about a large index by using
> pgstatindex, we get strange value of size and density.
> Because the values exceed int-max.
> ...
> I think that max_avail and free_space should be uint64.

Most places where we've dealt with this before, we use double, which is
guaranteed to be available whereas uint64 is not ...

            regards, tom lane

Re: Fix pgstatindex using for large indexes

От
Tatsuhito Kasahara
Дата:
Hi.

Tom Lane wrote:
>> I think that max_avail and free_space should be uint64.
> 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.

Best regards.

--
NTT OSS Center
Tatsuhito Kasahara

kasahara.tatsuhito _at_ oss.ntt.co.jp


*** postgresql-8.3.0.org/contrib/pgstattuple/pgstatindex.c    2007-11-16 06:14:31.000000000 +0900
--- postgresql-8.3.0/contrib/pgstattuple/pgstatindex.c    2008-02-24 19:35:09.000000000 +0900
***************
*** 68,75 ****
      uint32        empty_pages;
      uint32        deleted_pages;

!     uint32        max_avail;
!     uint32        free_space;

      uint32        fragments;
  }    BTIndexStat;
--- 68,75 ----
      uint32        empty_pages;
      uint32        deleted_pages;

!     double        max_avail;
!     double        free_space;

      uint32        fragments;
  }    BTIndexStat;
***************
*** 87,94 ****
      Relation    rel;
      RangeVar   *relrv;
      Datum        result;
!     uint32        nblocks;
!     uint32        blkno;
      BTIndexStat indexStat;

      if (!superuser())
--- 87,94 ----
      Relation    rel;
      RangeVar   *relrv;
      Datum        result;
!     BlockNumber    nblocks;
!     BlockNumber    blkno;
      BTIndexStat indexStat;

      if (!superuser())
***************
*** 207,231 ****
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.level);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", (indexStat.root_pages +
!                                          indexStat.leaf_pages +
!                                          indexStat.internal_pages +
!                                          indexStat.deleted_pages +
!                                          indexStat.empty_pages) * BLCKSZ);
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.root_blkno);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.internal_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.leaf_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.empty_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.deleted_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", 100.0 - (float) indexStat.free_space / (float) indexStat.max_avail *
100.0);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", (float) indexStat.fragments / (float) indexStat.leaf_pages * 100.0);

          tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
                                         values);
--- 207,231 ----
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.level);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.0f", ( (double) indexStat.root_pages +
!                                         (double) indexStat.leaf_pages +
!                                         (double) indexStat.internal_pages +
!                                         (double) indexStat.deleted_pages +
!                                         (double) indexStat.empty_pages) * BLCKSZ);
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.root_blkno);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.internal_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.leaf_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.empty_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.deleted_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", 100.0 - indexStat.free_space / indexStat.max_avail * 100.0);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", (double) indexStat.fragments / (double) indexStat.leaf_pages * 100.0);

          tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
                                         values);

Re: Fix pgstatindex using for large indexes

От
Tatsuhito Kasahara
Дата:
Tatsuhito Kasahara wrote:
> I fix the patch.
Oops, I forgot to attach the patch for pgstattuple.sql.
I send it again.

Best regards.

--
NTT OSS Center
Tatsuhito Kasahara

kasahara.tatsuhito _at_ oss.ntt.co.jp
diff -crN postgresql-8.3.0.org/contrib/pgstattuple/pgstatindex.c postgresql-8.3.0/contrib/pgstattuple/pgstatindex.c
*** postgresql-8.3.0.org/contrib/pgstattuple/pgstatindex.c    2007-11-16 06:14:31.000000000 +0900
--- postgresql-8.3.0/contrib/pgstattuple/pgstatindex.c    2008-02-24 19:35:09.000000000 +0900
***************
*** 68,75 ****
      uint32        empty_pages;
      uint32        deleted_pages;

!     uint32        max_avail;
!     uint32        free_space;

      uint32        fragments;
  }    BTIndexStat;
--- 68,75 ----
      uint32        empty_pages;
      uint32        deleted_pages;

!     double        max_avail;
!     double        free_space;

      uint32        fragments;
  }    BTIndexStat;
***************
*** 87,94 ****
      Relation    rel;
      RangeVar   *relrv;
      Datum        result;
!     uint32        nblocks;
!     uint32        blkno;
      BTIndexStat indexStat;

      if (!superuser())
--- 87,94 ----
      Relation    rel;
      RangeVar   *relrv;
      Datum        result;
!     BlockNumber    nblocks;
!     BlockNumber    blkno;
      BTIndexStat indexStat;

      if (!superuser())
***************
*** 207,231 ****
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.level);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", (indexStat.root_pages +
!                                          indexStat.leaf_pages +
!                                          indexStat.internal_pages +
!                                          indexStat.deleted_pages +
!                                          indexStat.empty_pages) * BLCKSZ);
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.root_blkno);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.internal_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.leaf_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.empty_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%d", indexStat.deleted_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", 100.0 - (float) indexStat.free_space / (float) indexStat.max_avail *
100.0);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", (float) indexStat.fragments / (float) indexStat.leaf_pages * 100.0);

          tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
                                         values);
--- 207,231 ----
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.level);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.0f", ( (double) indexStat.root_pages +
!                                         (double) indexStat.leaf_pages +
!                                         (double) indexStat.internal_pages +
!                                         (double) indexStat.deleted_pages +
!                                         (double) indexStat.empty_pages) * BLCKSZ);
          values[j] = palloc(32);
          snprintf(values[j++], 32, "%d", indexStat.root_blkno);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.internal_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.leaf_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.empty_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%u", indexStat.deleted_pages);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", 100.0 - indexStat.free_space / indexStat.max_avail * 100.0);
          values[j] = palloc(32);
!         snprintf(values[j++], 32, "%.2f", (double) indexStat.fragments / (double) indexStat.leaf_pages * 100.0);

          tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
                                         values);
diff -crN postgresql-8.3.0.org/contrib/pgstattuple/pgstattuple.sql.in
postgresql-8.3.0/contrib/pgstattuple/pgstattuple.sql.in
*** postgresql-8.3.0.org/contrib/pgstattuple/pgstattuple.sql.in    2007-11-13 13:24:28.000000000 +0900
--- postgresql-8.3.0/contrib/pgstattuple/pgstattuple.sql.in    2008-02-21 21:33:02.000000000 +0900
***************
*** 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;


Re: Fix pgstatindex using for large indexes

От
Zdenek Kotala
Дата:
Tom Lane napsal(a):
> Tatsuhito Kasahara <kasahara.tatsuhito@oss.ntt.co.jp> writes:
>> In pgstatindex.c and pgstattuple.sql, some variables are defined with
>> int type. So when we try to get informations about a large index by using
>> pgstatindex, we get strange value of size and density.
>> Because the values exceed int-max.
>> ...
>> I think that max_avail and free_space should be uint64.
>
> Most places where we've dealt with this before, we use double, which is
> guaranteed to be available whereas uint64 is not ...

Is this requirement still valid? Is there any currently supported platform which
does not have uint64? IIRC we are going to change datetime to integer for 8.4.
By my opinion uint64 is suitable for head and probably for 8.2 and 8.3 as well.


64bit integer is already used on many places: e.g.

./commands/copy.c
./tcop/utility.c.
./optimizer/plan/planner.c



        Zdenek

Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> Tom Lane napsal(a):
>> Most places where we've dealt with this before, we use double, which is
>> guaranteed to be available whereas uint64 is not ...

> Is this requirement still valid?

Yes.

> Is there any currently supported platform which
> does not have uint64?

I don't know, and neither do you.

> IIRC we are going to change datetime to integer for 8.4.

We are going to change the *default* to integer.  It will still be
possible to select float datetimes for use on platforms without working
int64.  There is not any core functionality that will fail completely
without int64, and none will be introduced anytime soon if I have
anything to say about it.

Now having said that, there are places that use int64 with the
understanding that it might degrade to int32, and that that will be
good enough --- the statistics counters are an example.  However,
the only point of the patch being presented for pgstatindex is to
be able to count higher than 2^32, so ISTM we may as well make it
use double.  There isn't any particular reason it *shouldn't* be
coded that way, AFAICS, and there is precedent in that VACUUM/ANALYZE
use doubles for similar purposes.

            regards, tom lane

Re: Fix pgstatindex using for large indexes

От
Andrew Dunstan
Дата:

Tom Lane wrote:
>
>> Is there any currently supported platform which
>> does not have uint64?
>>
>
> I don't know, and neither do you.
>
>
>

Maybe we should look at some reasonable way of getting the info out of a
compiled instance. How about if we get pg_config to output the value of
INT64_IS_BUSTED? Then we could add a step to the buildfarm client to
catch all the output from pg_config.

cheers

andrew

Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>>> Is there any currently supported platform which
>>> does not have uint64?
>>
>> I don't know, and neither do you.

> Maybe we should look at some reasonable way of getting the info out of a
> compiled instance. How about if we get pg_config to output the value of
> INT64_IS_BUSTED?

We know all the buildfarm machines have working int64, because they'd
fail the bigint regression test if not.  That's not the point here.

            regards, tom lane

Re: Fix pgstatindex using for large indexes

От
"Joshua D. Drake"
Дата:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 25 Feb 2008 11:21:18 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> > IIRC we are going to change datetime to integer for 8.4. 
> 
> We are going to change the *default* to integer.

Thank goodness. Now I can stop recompiling rpms. Thanks for this
- -hackers.

Joshua D. Drake



- -- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL SPI Liaison | SPI Director |  PostgreSQL political pundit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHwvkdATb/zqfZUUQRAnl8AJ4ou850ROztMxHZyGeUaD/uXQRMPACeMN9x
72FC2K3hKKV/Aq+FPWI8UJ4=
=EeIA
-----END PGP SIGNATURE-----

Re: Fix pgstatindex using for large indexes

От
"Florian G. Pflug"
Дата:
Tom Lane wrote:
> Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
>> Tom Lane napsal(a):
>>> Most places where we've dealt with this before, we use double, which is
>>> guaranteed to be available whereas uint64 is not ...
>
>> Is this requirement still valid?
>
> Yes.
Maybe we should just bite the bullet, and implement int64 emulation
for platforms that don't provide one? I was thinking that something
like "typedef struct { int32 low, int32 high } int64", plus a few
Macros for basic arithmetic should do the trick.

Not particularly rewarding work, given that all major platforms *do*
support int64 - but it'd prevent the discussion that starts everytime
someone proposes a patch that depends on int64.

regards, Florian Pflug

Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
"Florian G. Pflug" <fgp@phlo.org> writes:
> Maybe we should just bite the bullet, and implement int64 emulation
> for platforms that don't provide one?

Why?  Workarounds such as "use double where needed" have served us
perfectly fine so far, with far less effort and notational ugliness
than this would involve.

There will come a time where either there's a really good reason to rely
on int64, or we feel that it's moot because any platform without int64
is certainly dead anyway.  I'm not sure how far off that time is, but
it's probably some fairly small number of years.  My position is simply
that pgstattuple does not present a reason to make that decision today,
especially not when making it rely on int64 is at variance with the
coding method already in use in related parts of the core backend.

            regards, tom lane

Re: Fix pgstatindex using for large indexes

От
Zdenek Kotala
Дата:
Tom Lane napsal(a):
> "Florian G. Pflug" <fgp@phlo.org> writes:
>> Maybe we should just bite the bullet, and implement int64 emulation
>> for platforms that don't provide one?
>
> Why?  Workarounds such as "use double where needed" have served us
> perfectly fine so far, with far less effort and notational ugliness
> than this would involve.

Disadvantage of double usage is in CPUs. Current CPUs are perfectly
optimized for int operation but there are limitation for floating point
operatim. For example T1 (niagara) has only one FP unit per whole CPU
(T2 has one FP per core) and AMD64 has three parallel execution unit for
microcode  but only one for FP and FP shares registry with MMX unit.
And operation system has more work with saving FP register and so on...
Not all these limitations are related to PostgreSQL but it is good to know.

By my opinion avoid double is important in critical(bottleneck) places.
How you mentioned double is OK for pgstattuple.


> There will come a time where either there's a really good reason to rely
> on int64, or we feel that it's moot because any platform without int64
> is certainly dead anyway.  I'm not sure how far off that time is, but
> it's probably some fairly small number of years.

I don't think support more than five years older platform makes sense
for new version of PostgreSQL. Very often users buy a new hardware for
new database. IIRC all platform (x86, SPARC, MIPS, ALPHA, PARISC ...) do
not have problem with 64bit for longer time.

        Zdenek




Re: Fix pgstatindex using for large indexes

От
David Fetter
Дата:
On Mon, Feb 25, 2008 at 11:50:11AM -0500, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Tom Lane wrote:
> >>> Is there any currently supported platform which does not have
> >>> uint64?
> >>
> >> I don't know, and neither do you.
>
> > Maybe we should look at some reasonable way of getting the info
> > out of a compiled instance. How about if we get pg_config to
> > output the value of INT64_IS_BUSTED?
>
> We know all the buildfarm machines have working int64, because
> they'd fail the bigint regression test if not.  That's not the point
> here.

If we don't have buildfarm coverage for machines where INT64_IS_BUSTED,
how do we know we support those architectures at all?

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Fix pgstatindex using for large indexes

От
Bruce Momjian
Дата:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Tatsuhito Kasahara wrote:
> Hi.
>
> Tom Lane wrote:
> >> I think that max_avail and free_space should be uint64.
> > 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.
>
> Best regards.
>
> --
> NTT OSS Center
> Tatsuhito Kasahara
>
> kasahara.tatsuhito _at_ oss.ntt.co.jp
>
>

> *** postgresql-8.3.0.org/contrib/pgstattuple/pgstatindex.c    2007-11-16 06:14:31.000000000 +0900
> --- postgresql-8.3.0/contrib/pgstattuple/pgstatindex.c    2008-02-24 19:35:09.000000000 +0900
> ***************
> *** 68,75 ****
>       uint32        empty_pages;
>       uint32        deleted_pages;
>
> !     uint32        max_avail;
> !     uint32        free_space;
>
>       uint32        fragments;
>   }    BTIndexStat;
> --- 68,75 ----
>       uint32        empty_pages;
>       uint32        deleted_pages;
>
> !     double        max_avail;
> !     double        free_space;
>
>       uint32        fragments;
>   }    BTIndexStat;
> ***************
> *** 87,94 ****
>       Relation    rel;
>       RangeVar   *relrv;
>       Datum        result;
> !     uint32        nblocks;
> !     uint32        blkno;
>       BTIndexStat indexStat;
>
>       if (!superuser())
> --- 87,94 ----
>       Relation    rel;
>       RangeVar   *relrv;
>       Datum        result;
> !     BlockNumber    nblocks;
> !     BlockNumber    blkno;
>       BTIndexStat indexStat;
>
>       if (!superuser())
> ***************
> *** 207,231 ****
>           values[j] = palloc(32);
>           snprintf(values[j++], 32, "%d", indexStat.level);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%d", (indexStat.root_pages +
> !                                          indexStat.leaf_pages +
> !                                          indexStat.internal_pages +
> !                                          indexStat.deleted_pages +
> !                                          indexStat.empty_pages) * BLCKSZ);
>           values[j] = palloc(32);
>           snprintf(values[j++], 32, "%d", indexStat.root_blkno);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%d", indexStat.internal_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%d", indexStat.leaf_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%d", indexStat.empty_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%d", indexStat.deleted_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%.2f", 100.0 - (float) indexStat.free_space / (float) indexStat.max_avail *
100.0);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%.2f", (float) indexStat.fragments / (float) indexStat.leaf_pages * 100.0);
>
>           tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
>                                          values);
> --- 207,231 ----
>           values[j] = palloc(32);
>           snprintf(values[j++], 32, "%d", indexStat.level);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%.0f", ( (double) indexStat.root_pages +
> !                                         (double) indexStat.leaf_pages +
> !                                         (double) indexStat.internal_pages +
> !                                         (double) indexStat.deleted_pages +
> !                                         (double) indexStat.empty_pages) * BLCKSZ);
>           values[j] = palloc(32);
>           snprintf(values[j++], 32, "%d", indexStat.root_blkno);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%u", indexStat.internal_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%u", indexStat.leaf_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%u", indexStat.empty_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%u", indexStat.deleted_pages);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%.2f", 100.0 - indexStat.free_space / indexStat.max_avail * 100.0);
>           values[j] = palloc(32);
> !         snprintf(values[j++], 32, "%.2f", (double) indexStat.fragments / (double) indexStat.leaf_pages * 100.0);
>
>           tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
>                                          values);
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://postgres.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: Fix pgstatindex using for large indexes

От
Alvaro Herrera
Дата:
Tatsuhito Kasahara wrote:
> Tatsuhito Kasahara wrote:
> > I fix the patch.
> Oops, I forgot to attach the patch for pgstattuple.sql.
> I send it again.

Hmm, this followup patch is wrong though -- the SQL definition is still
using BIGINT where it should be using double.  And the other changes to
use BIGINT where the original values were int4 seem unnecessary.

One thing I'm not clear about is the change from %d to %u to represent
int4 values.  Since the SQL datatype is signed, this can't really work,
now, can it?

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
Tatsuhito Kasahara <kasahara.tatsuhito@oss.ntt.co.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

Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Hmm, this followup patch is wrong though -- the SQL definition is still
> using BIGINT where it should be using double.  And the other changes to
> use BIGINT where the original values were int4 seem unnecessary.

I'm on this one now ...

            regards, tom lane

Re: Fix pgstatindex using for large indexes

От
Tom Lane
Дата:
I wrote:
> In short, I'm willing to drop my opposition to the original form
> of the patch.

Original version applied with some minor tweaks (notably, pg_relpages
had the same problem).

            regards, tom lane