Thread: Inconsistent use of relpages = -1

Inconsistent use of relpages = -1

From
Jeff Davis
Date:
As Corey discovered, and I re-discovered later, partitioned tables can
have relpages=-1:

https://www.postgresql.org/message-id/f4a0cf7975f1ad42a20fcc91be9e938a4f71259d.camel@j-davis.com

One problem is that the code (analyze.c:680) is a bit unclear because
it implicitly casts back and forth between signed and unsigned. If we
really mean InvalidBlockNumber, then we should use that instead of -1,
and then explicitly cast to an integer for storage in the catalog.

Another problem is that it's inconsistent: sometimes it's 0 (before
analyze) and sometimes -1. Views also don't have any storage, but
relpages are always 0.

And lastly, it's undocumented: if -1 is allowable, it should be in the
public docs for pg_class.

I don't see any obvious reason that -1 is better than 0, or any code
that checks for it, so I'm inclined to just use zero instead.

Thoughts?

Regards,
    Jeff Davis




Re: Inconsistent use of relpages = -1

From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes:
> As Corey discovered, and I re-discovered later, partitioned tables can
> have relpages=-1:
> ...
> Another problem is that it's inconsistent: sometimes it's 0 (before
> analyze) and sometimes -1. Views also don't have any storage, but
> relpages are always 0.
> ...
> I don't see any obvious reason that -1 is better than 0, or any code
> that checks for it, so I'm inclined to just use zero instead.

Hmm.  relpages = 0 should mean that the relation is (thought to be)
of size zero, which is certainly true for the partitioned table itself,
except that that reasoning should also mean that reltuples is zero.
But we don't do that:

regression=# select relname, relkind, relpages,reltuples from pg_class where relpages < 0;
    relname     | relkind | relpages | reltuples
----------------+---------+----------+-----------
 past_parted    | p       |       -1 |         0
 prt1           | p       |       -1 |       300
 pagg_tab       | p       |       -1 |      3000
 prt2           | p       |       -1 |       200
 pagg_tab1      | p       |       -1 |       150
 pagg_tab2      | p       |       -1 |       100
 prt1_e         | p       |       -1 |       300
 prt2_e         | p       |       -1 |       200
 pagg_tab_m     | p       |       -1 |      3000
 pagg_tab_ml    | p       |       -1 |     30000
 prt1_m         | p       |       -1 |       300
 prt2_m         | p       |       -1 |       200
 pagg_tab_ml_p2 | p       |       -1 |      8000
 pagg_tab_ml_p3 | p       |       -1 |     10000
 pagg_tab_para  | p       |       -1 |     30000
 plt1           | p       |       -1 |       300
 plt2           | p       |       -1 |       200
 plt1_e         | p       |       -1 |       300
 pht1           | p       |       -1 |       300
 pht2           | p       |       -1 |       200
 pht1_e         | p       |       -1 |       150
 prt1_l         | p       |       -1 |       300
 prt1_l_p2      | p       |       -1 |       125
 prt1_l_p3      | p       |       -1 |        50
 prt2_l         | p       |       -1 |       200
 prt2_l_p2      | p       |       -1 |        83
 prt2_l_p3      | p       |       -1 |        33
 prt1_n         | p       |       -1 |       250
 prt2_n         | p       |       -1 |       300
 prt3_n         | p       |       -1 |         0
 prt4_n         | p       |       -1 |       300
 alpha          | p       |       -1 |       360
 alpha_neg      | p       |       -1 |       180
 alpha_pos      | p       |       -1 |       180
 beta           | p       |       -1 |       360
 beta_neg       | p       |       -1 |       180
 beta_pos       | p       |       -1 |       180
(37 rows)

If we are going to put data into reltuples but not relpages,
I think I agree with setting relpages to -1 to signify
"unknown" (analogously to -1 for reltuples).  Otherwise it
looks like the table has infinite tuple density, which is
likely to bollix something somewhere.

I agree that not documenting this usage is bad, and not being
consistent about it is worse.  But I don't think switching
over to the previously-minority case will improve matters.

            regards, tom lane



Re: Inconsistent use of relpages = -1

From
Laurenz Albe
Date:
On Tue, 2024-10-22 at 10:41 -0700, Jeff Davis wrote:
> I attached a patch that creates partitioned tables with relpages=-1,
> and updates the docs.

Does this need any changes for pg_upgrade?

Yours,
Laurenz Albe



Re: Inconsistent use of relpages = -1

From
Jeff Davis
Date:
On Wed, 2024-10-23 at 04:47 +0200, Laurenz Albe wrote:
> On Tue, 2024-10-22 at 10:41 -0700, Jeff Davis wrote:
> > I attached a patch that creates partitioned tables with relpages=-
> > 1,
> > and updates the docs.
>
> Does this need any changes for pg_upgrade?

pg_upgrade would go through the same DDL path, so I expect it would be
set to -1 in the upgraded cluster anyway. Are you seeing something
different?

In any case, the change is just meant to improve consistency and
documentation. I didn't intend to create a hard guarantee that it would
always be -1, so perhaps I should make the documentation more vague
with "may be" instead of "is"?

It bothers me somewhat that views still have relpages=0, because they
also don't have storage. Thoughts?

Regards,
    Jeff Davis




Re: Inconsistent use of relpages = -1

From
Tom Lane
Date:
Jeff Davis <pgsql@j-davis.com> writes:
> It bothers me somewhat that views still have relpages=0, because they
> also don't have storage. Thoughts?

I think it's fine, because they also have reltuples=0.  The
combination of those two zeroes indicates ignorance as to
the rel's contents.  If either field is not zero, though, then
the combination ought to be sensible.

            regards, tom lane



Re: Inconsistent use of relpages = -1

From
Laurenz Albe
Date:
On Wed, 2024-10-23 at 10:05 -0700, Jeff Davis wrote:
> On Wed, 2024-10-23 at 04:47 +0200, Laurenz Albe wrote:
> > On Tue, 2024-10-22 at 10:41 -0700, Jeff Davis wrote:
> > > I attached a patch that creates partitioned tables with relpages=-
> > > 1,
> > > and updates the docs.
> >
> > Does this need any changes for pg_upgrade?
>
> pg_upgrade would go through the same DDL path, so I expect it would be
> set to -1 in the upgraded cluster anyway. Are you seeing something
> different?

No; I was too lazy to check, so I asked.

> In any case, the change is just meant to improve consistency and
> documentation. I didn't intend to create a hard guarantee that it would
> always be -1, so perhaps I should make the documentation more vague
> with "may be" instead of "is"?

Reading the thread an Tom's comments again, I get the impression that there
are two states that we consider OK:

- "relpages" and "reltuples" are both 0
- "relpages" is -1 and "reltuples" is greater than 0

What you write above indicates that "relpages" = 0 and "reltuples" > 0
would also be acceptable.

As long as the code does not mistakenly assume that a partitioned table
is empty, and as long as the documentation is not confusing, anything
is fine with me.  Currently, I am still a bit confused.

Yours,
Laurenz Albe



Re: Inconsistent use of relpages = -1

From
Jeff Davis
Date:
On Thu, 2024-10-24 at 05:01 +0300, Laurenz Albe wrote:
> What you write above indicates that "relpages" = 0 and "reltuples" >
> 0
> would also be acceptable.

As Tom pointed out, that creates a risk that it's interpreted as
infinite tuple denisity.

The only functional change in my patch is to create the partitioned
table with relpages=-1 to be more consistent with the value after it's
analyzed.

Regards,
    Jeff Davis



Re: Inconsistent use of relpages = -1

From
Laurenz Albe
Date:
On Thu, 2024-10-24 at 08:03 -0700, Jeff Davis wrote:
> On Thu, 2024-10-24 at 05:01 +0300, Laurenz Albe wrote:
> > What you write above indicates that "relpages" = 0 and "reltuples" >
> > 0
> > would also be acceptable.
>
> As Tom pointed out, that creates a risk that it's interpreted as
> infinite tuple denisity.
>
> The only functional change in my patch is to create the partitioned
> table with relpages=-1 to be more consistent with the value after it's
> analyzed.

Great, then it cannot be wrong.  Sorry for the noise!

Yours,
Laurenz Albe