Horton

Thread

Commits

  1. Prevent integer overflow when forming tuple width estimates.

  2. compute_bitmap_pages' loop_count parameter should be double not int.

  1. BUG #18247: Integer overflow leads to negative width

    The Post Office <noreply@postgresql.org> — 2023-12-14T04:16:53Z

    The following bug has been logged on the website:

Bug reference:      18247
Logged by:          RekGRpth
Email address:      rekgrpth@gmail.com
PostgreSQL version: 16.1
Operating system:   docker alpine
Description:        

CREATE TABLE t (
    c01 character(10485760),
    c02 character(10485760),
    c03 character(10485760),
    c04 character(10485760),
    c05 character(10485760),
    c06 character(10485760),
    c07 character(10485760),
    c08 character(10485760),
    c09 character(10485760),
    c10 character(10485760),
    c11 character(10485760),
    c12 character(10485760),
    c13 character(10485760),
    c14 character(10485760),
    c15 character(10485760),
    c16 character(10485760),
    c17 character(10485760),
    c18 character(10485760),
    c19 character(10485760),
    c20 character(10485760),
    c21 character(10485760),
    c22 character(10485760),
    c23 character(10485760),
    c24 character(10485760),
    c25 character(10485760),
    c26 character(10485760),
    c27 character(10485760),
    c28 character(10485760),
    c29 character(10485760),
    c30 character(10485760),
    c31 character(10485760),
    c32 character(10485760),
    c33 character(10485760),
    c34 character(10485760),
    c35 character(10485760),
    c36 character(10485760),
    c37 character(10485760),
    c38 character(10485760),
    c39 character(10485760),
    c40 character(10485760),
    c41 character(10485760),
    c42 character(10485760),
    c43 character(10485760),
    c44 character(10485760),
    c45 character(10485760),
    c46 character(10485760),
    c47 character(10485760),
    c48 character(10485760),
    c49 character(10485760),
    c50 character(10485760),
    c51 character(10485760),
    c52 character(10485760)
);
EXPLAIN SELECT * FROM t;
                         QUERY PLAN                         
------------------------------------------------------------
 Seq Scan on t  (cost=0.00..10.00 rows=1 width=-2113929008)
(1 row)

  2. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-14T10:34:44Z

    On Thu, Dec 14, 2023 at 5:29 PM PG Bug reporting form <
noreply@postgresql.org> wrote:

> EXPLAIN SELECT * FROM t;
>                          QUERY PLAN
> ------------------------------------------------------------
>  Seq Scan on t  (cost=0.00..10.00 rows=1 width=-2113929008)
> (1 row)


Interesting.  In an Assert-enabled build this query will cause the
Assert failure in set_rel_width().

    Assert(tuple_width >= 0);

Can we just error out when an overflow occurs?

Thanks
Richard

  3. Re: BUG #18247: Integer overflow leads to negative width

    Julien Rouhaud <rjuju123@gmail.com> — 2023-12-14T10:57:40Z

    On Thu, Dec 14, 2023 at 06:34:44PM +0800, Richard Guo wrote:
> On Thu, Dec 14, 2023 at 5:29 PM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>
> > EXPLAIN SELECT * FROM t;
> >                          QUERY PLAN
> > ------------------------------------------------------------
> >  Seq Scan on t  (cost=0.00..10.00 rows=1 width=-2113929008)
> > (1 row)
>
>
> Interesting.  In an Assert-enabled build this query will cause the
> Assert failure in set_rel_width().
>
>     Assert(tuple_width >= 0);
>
> Can we just error out when an overflow occurs?

I'm worried that it could have quite a broad impact, as the same problem could
easily arise in some more conventional cases, e.g. after joining some already
large relations.  I'm sure that some people already hit that in production
without looking at explain plans, and wouldn't be happy to see it broken during
the next update.

  4. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-14T14:43:45Z

    Richard Guo <guofenglinux@gmail.com> writes:
> On Thu, Dec 14, 2023 at 5:29 PM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>> EXPLAIN SELECT * FROM t;
>> QUERY PLAN
>> ------------------------------------------------------------
>> Seq Scan on t  (cost=0.00..10.00 rows=1 width=-2113929008)
>> (1 row)

> Can we just error out when an overflow occurs?

Probably better to clamp tuple width estimates to MaxAllocSize.
Anything larger would not correspond to reality anyhow.

			regards, tom lane

  5. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-15T02:28:10Z

    On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > On Thu, Dec 14, 2023 at 5:29 PM PG Bug reporting form <
> > noreply@postgresql.org> wrote:
> >> EXPLAIN SELECT * FROM t;
> >> QUERY PLAN
> >> ------------------------------------------------------------
> >> Seq Scan on t  (cost=0.00..10.00 rows=1 width=-2113929008)
> >> (1 row)
>
> > Can we just error out when an overflow occurs?
>
> Probably better to clamp tuple width estimates to MaxAllocSize.
> Anything larger would not correspond to reality anyhow.


Fair point.  How about the attached patch?

Thanks
Richard

  6. Re: BUG #18247: Integer overflow leads to negative width

    RekGRpth <rekgrpth@gmail.com> — 2023-12-15T03:13:44Z

    How bad would it be if, after overflowing, the width value was within
the allowed range?

пт, 15 дек. 2023 г. в 07:28, Richard Guo <guofenglinux@gmail.com>:
>
>
> On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Richard Guo <guofenglinux@gmail.com> writes:
>> > On Thu, Dec 14, 2023 at 5:29 PM PG Bug reporting form <
>> > noreply@postgresql.org> wrote:
>> >> EXPLAIN SELECT * FROM t;
>> >> QUERY PLAN
>> >> ------------------------------------------------------------
>> >> Seq Scan on t  (cost=0.00..10.00 rows=1 width=-2113929008)
>> >> (1 row)
>>
>> > Can we just error out when an overflow occurs?
>>
>> Probably better to clamp tuple width estimates to MaxAllocSize.
>> Anything larger would not correspond to reality anyhow.
>
>
> Fair point.  How about the attached patch?
>
> Thanks
> Richard

  7. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-15T03:20:21Z

    RekGRpth <rekgrpth@gmail.com> writes:
> How bad would it be if, after overflowing, the width value was within
> the allowed range?

Really pretty negligible, I should think.  You might get a not-great
choice of plan.  As Richard noted, there are some Asserts you could
hit; but not in a production build.

			regards, tom lane

  8. Re: BUG #18247: Integer overflow leads to negative width

    Alexander Law <exclusion@gmail.com> — 2023-12-15T06:00:00Z

    Hello Richard,

15.12.2023 05:28, Richard Guo wrote:
>
> Fair point.  How about the attached patch?

Your patch looks good to me, but maybe you would find it suitable to fix in
passing one more integer overflow in costsize.c?

Concretely, the query:
CREATE TABLE t(id int PRIMARY KEY, i int);
EXPLAIN (VERBOSE)
UPDATE t SET i = ni FROM (SELECT g id, 1 ni FROM generate_series(1, 2147483648) g) s WHERE t.id = s.id;

when executed with ubsan-enabled build, gives:
costsize.c:1017:12: runtime error: 2.14748e+09 is outside the range of representable values of type 'int'
     #0 0x5603325818e0 in cost_bitmap_heap_scan .../src/backend/optimizer/path/costsize.c:1017:12
     #1 0x5603326cc519 in create_bitmap_heap_path .../src/backend/optimizer/util/pathnode.c:1065:2
...

Without ubsan enabled, the query:
EXPLAIN (VERBOSE)
UPDATE t SET i = ni FROM (SELECT g id, 1 ni FROM generate_series(1, 2147483648) g) s WHERE t.id = s.id;

executed visually similar to:
EXPLAIN (VERBOSE, ANALYZE)
UPDATE t SET i = ni FROM (SELECT g id, 1 ni FROM generate_series(1, 2147483647) g) s WHERE t.id = s.id;

but quite longer:
  Update on public.t  (cost=60.85..27122613.04 rows=0 width=0) (actual time=225204.159..225204.162 rows=0 loops=1)
vs
  Update on public.t  (cost=60.85..27122613.03 rows=0 width=0) (actual time=153015.851..153015.852 rows=0 loops=1)

Best regards,
Alexander

  9. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-15T12:01:45Z

    On Fri, Dec 15, 2023 at 2:00 PM Alexander Lakhin <exclusion@gmail.com>
wrote:

> Your patch looks good to me, but maybe you would find it suitable to fix in
> passing one more integer overflow in costsize.c?
>
> Concretely, the query:
> CREATE TABLE t(id int PRIMARY KEY, i int);
> EXPLAIN (VERBOSE)
> UPDATE t SET i = ni FROM (SELECT g id, 1 ni FROM generate_series(1,
> 2147483648) g) s WHERE t.id = s.id;
>
> when executed with ubsan-enabled build, gives:
> costsize.c:1017:12: runtime error: 2.14748e+09 is outside the range of
> representable values of type 'int'
>     #0 0x5603325818e0 in cost_bitmap_heap_scan
> .../src/backend/optimizer/path/costsize.c:1017:12
>     #1 0x5603326cc519 in create_bitmap_heap_path
> .../src/backend/optimizer/util/pathnode.c:1065:2
>

Nice catch.  The overflow occurs when cost_bitmap_heap_scan() calls
compute_bitmap_pages(), and the loop_count parameter is converted from
double to int.  I wonder if we can change the loop_count parameter to be
double for compute_bitmap_pages() to avoid such overflow.

Thanks
Richard

  10. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-15T15:30:40Z

    Richard Guo <guofenglinux@gmail.com> writes:
> On Fri, Dec 15, 2023 at 2:00 PM Alexander Lakhin <exclusion@gmail.com>
>> Your patch looks good to me, but maybe you would find it suitable to fix in
>> passing one more integer overflow in costsize.c?

> Nice catch.  The overflow occurs when cost_bitmap_heap_scan() calls
> compute_bitmap_pages(), and the loop_count parameter is converted from
> double to int.  I wonder if we can change the loop_count parameter to be
> double for compute_bitmap_pages() to avoid such overflow.

Yeah.  Seems like a flat-out error in da08a6598: that logic had been
treating loop_count as double for years, and then when it was split
out into a subroutine, somebody carelessly declared the argument as
int.  (Even sillier, all the callers are trying to pass doubles.)

compute_bitmap_pages is substantially below par as to commenting,
too.

However, I'd be a bit uncomfortable about back-patching; since that
function is globally exposed, it's at least possible that some
extension is calling it and would see an ABI break.  Is it good enough
to fix this in HEAD?  I'd argue yes, given that a loop_count larger
than INT_MAX seems like a pretty improbable case.

			regards, tom lane

  11. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-15T15:43:03Z

    Richard Guo <guofenglinux@gmail.com> writes:
> On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Probably better to clamp tuple width estimates to MaxAllocSize.
>> Anything larger would not correspond to reality anyhow.

> Fair point.  How about the attached patch?

We'd need to hit at least build_joinrel_tlist too.  Not sure
offhand whether this is enough to cover upper-relation tlists.

As far as the specifics go, is it enough to clamp once?  I think
we'd either have to clamp after each addition, or change the
running-sum variables to double and clamp just before storing
into the final width field.  The latter seems probably
less error-prone in the long run.

Also, given that we'll need at least three copies of the clamp
rule, I wonder if it's worth inventing a function comparable
to clamp_row_est().

			regards, tom lane

  12. Re: BUG #18247: Integer overflow leads to negative width

    RekGRpth <rekgrpth@gmail.com> — 2023-12-16T15:24:49Z

    If the value itself is not important, but the comparison of values is
important, then maybe it is possible to non-dimensionalize these
values?

пт, 15 дек. 2023 г. в 20:43, Tom Lane <tgl@sss.pgh.pa.us>:
>
> Richard Guo <guofenglinux@gmail.com> writes:
> > On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Probably better to clamp tuple width estimates to MaxAllocSize.
> >> Anything larger would not correspond to reality anyhow.
>
> > Fair point.  How about the attached patch?
>
> We'd need to hit at least build_joinrel_tlist too.  Not sure
> offhand whether this is enough to cover upper-relation tlists.
>
> As far as the specifics go, is it enough to clamp once?  I think
> we'd either have to clamp after each addition, or change the
> running-sum variables to double and clamp just before storing
> into the final width field.  The latter seems probably
> less error-prone in the long run.
>
> Also, given that we'll need at least three copies of the clamp
> rule, I wonder if it's worth inventing a function comparable
> to clamp_row_est().
>
>                         regards, tom lane

  13. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-18T05:45:33Z

    On Fri, Dec 15, 2023 at 11:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > On Fri, Dec 15, 2023 at 2:00 PM Alexander Lakhin <exclusion@gmail.com>
> >> Your patch looks good to me, but maybe you would find it suitable to
> fix in
> >> passing one more integer overflow in costsize.c?
>
> > Nice catch.  The overflow occurs when cost_bitmap_heap_scan() calls
> > compute_bitmap_pages(), and the loop_count parameter is converted from
> > double to int.  I wonder if we can change the loop_count parameter to be
> > double for compute_bitmap_pages() to avoid such overflow.
>
> However, I'd be a bit uncomfortable about back-patching; since that
> function is globally exposed, it's at least possible that some
> extension is calling it and would see an ABI break.  Is it good enough
> to fix this in HEAD?  I'd argue yes, given that a loop_count larger
> than INT_MAX seems like a pretty improbable case.


I agree with you that it's good enough to fix this in HEAD.  The lack of
complaints from fields for so many years suggests that it's not a common
case to have loop_count larger than INT_MAX.

Thanks
Richard

  14. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-18T08:43:37Z

    On Fri, Dec 15, 2023 at 11:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Probably better to clamp tuple width estimates to MaxAllocSize.
> >> Anything larger would not correspond to reality anyhow.
>
> > Fair point.  How about the attached patch?
>
> We'd need to hit at least build_joinrel_tlist too.


Right.  And I think we also need to hit add_placeholders_to_joinrel.


>   Not sure
> offhand whether this is enough to cover upper-relation tlists.


I think we need to do the same to create_one_window_path.  For
intermediate WindowAggPaths, we need to add the WindowFuncs to their
output targets, and that would increase the width of the tlists.

Also, it is possible that there could be tuple-width overflow occurring
within function get_rel_data_width().  The return value of this function
is used to calculate density, or returned by get_relation_data_width().
So I wonder if we could change the return type of get_rel_data_width()
and get_relation_data_width() to be double, and handle the overflow in
the callers if needed.  But this may lead to ABI break.


> As far as the specifics go, is it enough to clamp once?  I think
> we'd either have to clamp after each addition, or change the
> running-sum variables to double and clamp just before storing
> into the final width field.  The latter seems probably
> less error-prone in the long run.


Agreed.


> Also, given that we'll need at least three copies of the clamp
> rule, I wonder if it's worth inventing a function comparable
> to clamp_row_est().


Yeah, I think we should do that.

Attached is an updated patch for all the changes.  It also changes the
loop_count parameter to be double for compute_bitmap_pages() in passing.
It does not improve the comments for compute_bitmap_pages() though.

Thanks
Richard

  15. Re: BUG #18247: Integer overflow leads to negative width

    Junwang Zhao <zhjwpku@gmail.com> — 2023-12-18T09:12:45Z

    On Mon, Dec 18, 2023 at 4:44 PM Richard Guo <guofenglinux@gmail.com> wrote:
>
>
> On Fri, Dec 15, 2023 at 11:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Richard Guo <guofenglinux@gmail.com> writes:
>> > On Thu, Dec 14, 2023 at 10:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> >> Probably better to clamp tuple width estimates to MaxAllocSize.
>> >> Anything larger would not correspond to reality anyhow.
>>
>> > Fair point.  How about the attached patch?
>>
>> We'd need to hit at least build_joinrel_tlist too.
>
>
> Right.  And I think we also need to hit add_placeholders_to_joinrel.
>
>>
>>   Not sure
>> offhand whether this is enough to cover upper-relation tlists.
>
>
> I think we need to do the same to create_one_window_path.  For
> intermediate WindowAggPaths, we need to add the WindowFuncs to their
> output targets, and that would increase the width of the tlists.
>
> Also, it is possible that there could be tuple-width overflow occurring
> within function get_rel_data_width().  The return value of this function
> is used to calculate density, or returned by get_relation_data_width().
> So I wonder if we could change the return type of get_rel_data_width()
> and get_relation_data_width() to be double, and handle the overflow in
> the callers if needed.  But this may lead to ABI break.
>
>>
>> As far as the specifics go, is it enough to clamp once?  I think
>> we'd either have to clamp after each addition, or change the
>> running-sum variables to double and clamp just before storing
>> into the final width field.  The latter seems probably
>> less error-prone in the long run.
>
>
> Agreed.
>
>>
>> Also, given that we'll need at least three copies of the clamp
>> rule, I wonder if it's worth inventing a function comparable
>> to clamp_row_est().
>
>
> Yeah, I think we should do that.
>
> Attached is an updated patch for all the changes.  It also changes the
> loop_count parameter to be double for compute_bitmap_pages() in passing.
> It does not improve the comments for compute_bitmap_pages() though.
>
I guess using double for the sum is in case of overflow of int64?
pg_class.relnatts
is of type int16, I guess it's not possible to overflow int64.

Using *int64* for the tuple_width is more intuitive, the
clamp_width_est will be:

+int
+clamp_width_est(int64 tuple_width)
+{
+ /*
+ * Clamp tuple-width estimate to MaxAllocSize in case it exceeds the limit
+ * or overflows.  Anything larger than MaxAllocSize would not align with
+ * reality.
+ */
+ if (tuple_width > MaxAllocSize)
+ tuple_width = MaxAllocSize;
+
+ return (int) tuple_width;
+}

> Thanks
> Richard



-- 
Regards
Junwang Zhao

  16. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-18T21:00:30Z

    Junwang Zhao <zhjwpku@gmail.com> writes:
> I guess using double for the sum is in case of overflow of int64?
> pg_class.relnatts
> is of type int16, I guess it's not possible to overflow int64.

Not sure what I think about that.  The main advantage of double is
that you definitely don't need to worry about overflow.  It's
probably about the same speed either way on modern hardware.
(On 32-bit hardware it's quite likely that double is faster,
since even 32-bit CPUs tend to have hardware floating point.)
I left it as double for the moment, but we could discuss that
some more.

Some review comments:

* clamp_width_est doesn't need to check isinf, since the
comparison will work just fine.  It does need to check isnan,
unless we switch to int64.

* We can fold the Assert about width >= 0 into clamp_width_est
too; it fits naturally there and saves code in the callers.

* I did not like changing the output type of get_rel_data_width;
it seems better to me to have it do clamp_width_est internally.
For one reason, several of the call sites are relying on
doing integer division, which the v2 patch might break.

The attached v3 fixes those things and makes some cosmetic
changes (mostly comments).

			regards, tom lane

  17. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-19T03:32:29Z

    On Tue, Dec 19, 2023 at 5:00 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Some review comments:
>
> * clamp_width_est doesn't need to check isinf, since the
> comparison will work just fine.  It does need to check isnan,
> unless we switch to int64.
>
> * We can fold the Assert about width >= 0 into clamp_width_est
> too; it fits naturally there and saves code in the callers.
>
> * I did not like changing the output type of get_rel_data_width;
> it seems better to me to have it do clamp_width_est internally.
> For one reason, several of the call sites are relying on
> doing integer division, which the v2 patch might break.
>
> The attached v3 fixes those things and makes some cosmetic
> changes (mostly comments).


I agree with all the comments made.  I have also examined other places
where the width field is assigned, and I think we've covered all cases.
So the v3 patch is in good shape to me.

Thanks
Richard

  18. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-19T04:08:57Z

    Richard Guo <guofenglinux@gmail.com> writes:
> I agree with all the comments made.  I have also examined other places
> where the width field is assigned, and I think we've covered all cases.
> So the v3 patch is in good shape to me.

Thanks for looking!  Do you have an opinion about the int64-vs-double
question?

			regards, tom lane

  19. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-19T05:53:59Z

    On Tue, Dec 19, 2023 at 12:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > I agree with all the comments made.  I have also examined other places
> > where the width field is assigned, and I think we've covered all cases.
> > So the v3 patch is in good shape to me.
>
> Thanks for looking!  Do you have an opinion about the int64-vs-double
> question?


To be honest, I don't have a preference on which one is better.  I think
double is good enough for now as we don't need to worry about overflow
with it.

Thanks
Richard

  20. Re: BUG #18247: Integer overflow leads to negative width

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-12-19T15:22:20Z

    Richard Guo <guofenglinux@gmail.com> writes:
> On Tue, Dec 19, 2023 at 12:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thanks for looking!  Do you have an opinion about the int64-vs-double
>> question?

> To be honest, I don't have a preference on which one is better.  I think
> double is good enough for now as we don't need to worry about overflow
> with it.

After sleeping on it, I'm coming around to the idea that int64 will
be better.  The argument that convinces me is that using int64
provides a datatype-based clue that we are working with a width
and not a row count, cost, or selectivity number.  I don't feel
a need to go as far as invent a typedef alias like Cardinality;
but plain "double" in the planner tends to be a rowcount estimate,
which is not what we want people to think of.

I'll make that change and push it.

BTW, I think it's sufficient to fix this in HEAD.  The troublesome
example seems quite artificial to me, and we've not heard field
reports suggesting that people have had real problems here.

			regards, tom lane

  21. Re: BUG #18247: Integer overflow leads to negative width

    Richard Guo <guofenglinux@gmail.com> — 2023-12-20T02:05:06Z

    On Tue, Dec 19, 2023 at 11:22 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Richard Guo <guofenglinux@gmail.com> writes:
> > On Tue, Dec 19, 2023 at 12:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Thanks for looking!  Do you have an opinion about the int64-vs-double
> >> question?
>
> > To be honest, I don't have a preference on which one is better.  I think
> > double is good enough for now as we don't need to worry about overflow
> > with it.
>
> After sleeping on it, I'm coming around to the idea that int64 will
> be better.  The argument that convinces me is that using int64
> provides a datatype-based clue that we are working with a width
> and not a row count, cost, or selectivity number.  I don't feel
> a need to go as far as invent a typedef alias like Cardinality;
> but plain "double" in the planner tends to be a rowcount estimate,
> which is not what we want people to think of.


Fair point.


> I'll make that change and push it.


Thanks for the change and pushing!


> BTW, I think it's sufficient to fix this in HEAD.  The troublesome
> example seems quite artificial to me, and we've not heard field
> reports suggesting that people have had real problems here.


Agreed.

Thanks
Richard