Re: Signed vs. Unsigned (some) - Mailing list pgsql-hackers
| From | Ranier Vilela |
|---|---|
| Subject | Re: Signed vs. Unsigned (some) |
| Date | |
| Msg-id | CAEudQApD0Gy5dVD4gtWrzF_3kiiriQ3FKr-RJBAiphjmwBsFOQ@mail.gmail.com Whole thread Raw |
| In response to | Signed vs. Unsigned (some) (Ranier Vilela <ranier.vf@gmail.com>) |
| List | pgsql-hackers |
Em sex., 2 de jul. de 2021 às 13:29, Alvaro Herrera <alvherre@alvh.no-ip.org> escreveu:
On 2021-Jul-02, Justin Pryzby wrote:
> On Fri, Jul 02, 2021 at 12:09:23PM +0200, Peter Eisentraut wrote:
> > The use of InvalidBlockNumber with vac_update_relstats() looks a bit fishy
> > to me. We are using in the same call 0 as the default for
> > num_all_visible_pages, and we generally elsewhere also use 0 as the starting
> > value for relpages, so it's not clear to me why it should be -1 or
> > InvalidBlockNumber here. I'd rather leave it "slightly wrong" for now so it
> > can be checked again.
> |commit 0e69f705cc1a3df273b38c9883fb5765991e04fe
> |Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
> |Date: Fri Apr 9 11:29:08 2021 -0400
> |
> | Set pg_class.reltuples for partitioned tables
>
> 3d35 also affects partitioned tables, and 0e69 appears to do the right thing by
> preserving relpages=-1 during auto-analyze.
I suppose the question is what is the value used for. BlockNumber is
typedef'd uint32, an unsigned variable, so using -1 for it is quite
fishy. The weird thing is that in vac_update_relstats we cast it to
(int32) when storing it in the pg_class tuple, so that's quite fishy
too.
What we really want is for table_block_relation_estimate_size to work
properly. What that does is get the signed-int32 value from pg_class
and cast it back to BlockNumber. If that assignment gets -1 again, then
it's all fine. I didn't test it.
It seems to me that it is happening, but it is risky to make comparisons between different types.
1)
#define InvalidBlockNumber 0xFFFFFFFF
int main()
{
unsigned int num_pages;
int rel_pages;
num_pages = -1;
rel_pages = (int) num_pages;
printf("num_pages = %u\n", num_pages);
printf("rel_pages = %d\n", rel_pages);
printf("(num_pages == InvalidBlockNumber) => %u\n", (num_pages == InvalidBlockNumber));
printf("(rel_pages == InvalidBlockNumber) => %u\n", (rel_pages == InvalidBlockNumber));
}
int main()
{
unsigned int num_pages;
int rel_pages;
num_pages = -1;
rel_pages = (int) num_pages;
printf("num_pages = %u\n", num_pages);
printf("rel_pages = %d\n", rel_pages);
printf("(num_pages == InvalidBlockNumber) => %u\n", (num_pages == InvalidBlockNumber));
printf("(rel_pages == InvalidBlockNumber) => %u\n", (rel_pages == InvalidBlockNumber));
}
num_pages = 4294967295
rel_pages = -1
(num_pages == InvalidBlockNumber) => 1
(rel_pages == InvalidBlockNumber) => 1 /* 17:68: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] */
rel_pages = -1
(num_pages == InvalidBlockNumber) => 1
(rel_pages == InvalidBlockNumber) => 1 /* 17:68: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] */
If num_pages is promoted to uint64 and rel_pages to int64:
2)
#define InvalidBlockNumber 0xFFFFFFFF
int main()
{
unsigned long int num_pages;
long int rel_pages;
num_pages = -1;
rel_pages = (int) num_pages;
printf("num_pages = %lu\n", num_pages);
printf("rel_pages = %ld\n", rel_pages);
printf("(num_pages == InvalidBlockNumber) => %u\n", (num_pages == InvalidBlockNumber));
printf("(rel_pages == InvalidBlockNumber) => %u\n", (rel_pages == InvalidBlockNumber));
}
int main()
{
unsigned long int num_pages;
long int rel_pages;
num_pages = -1;
rel_pages = (int) num_pages;
printf("num_pages = %lu\n", num_pages);
printf("rel_pages = %ld\n", rel_pages);
printf("(num_pages == InvalidBlockNumber) => %u\n", (num_pages == InvalidBlockNumber));
printf("(rel_pages == InvalidBlockNumber) => %u\n", (rel_pages == InvalidBlockNumber));
}
rel_pages = -1
(num_pages == InvalidBlockNumber) => 0
(rel_pages == InvalidBlockNumber) => 0 /* 17:68: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] */
As Kyotaro said:
"they might be different if we forget to widen the constant
when widening the types"
when widening the types"
regards,
Ranier Vilela
pgsql-hackers by date: