public inbox for [email protected]  
help / color / mirror / Atom feed
From: Robert Haas <[email protected]>
To: Ilia Evdokimov <[email protected]>
Cc: Andrei Lepikhov <[email protected]>
Cc: [email protected]
Cc: Tom Lane <[email protected]>
Cc: Matheus Alcantara <[email protected]>
Cc: Guillaume Lelarge <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Ibrar Ahmed <[email protected]>
Cc: Gregory Stark (as CFM) <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Peter Geoghegan <[email protected]>
Cc: vignesh C <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Alena Rybakina <[email protected]>
Subject: Re: explain analyze rows=%.0f
Date: Mon, 17 Feb 2025 09:19:43 -0500
Message-ID: <CA+TgmobVWYnYDj1HZ--iqeKx+n7zTgtYCar+YfZ0V6wiX-t0Mg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+TgmoZv8DvmeQHQAX+rkn68id2N+DoUhFZB36d-_w1tyAeqeA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAFY6G8f6Sdu9cvfF2NYWWZMHD0ecTCPuNT7VktPH5NwxY7hcog@mail.gmail.com>
	<[email protected]>
	<CAFY6G8dgp494Xi2+VUUyM2CvuSQ+3nnzWi44gURWbCHjEWW89g@mail.gmail.com>
	<[email protected]>
	<CA+Tgmobgkh5CW=4Xpejw9nR6Onv8SZh0nnr+=HmNAFyyHZb0eg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CA+TgmobT9gPQLtw143oGnKVNtaOrTDOy+ZbLfe18MVZ9POjaWg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CA+TgmoYzawdq40C5cN==LRPBvQChZ7t5XxH6wv+zMxnyM7LjBQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmobfeePej0uNfbLy0v6jUrB3D9m1Kh+QQ40_-A9KQjkqUw@mail.gmail.com>
	<[email protected]>
	<[email protected]>

On Mon, Feb 17, 2025 at 3:08 AM Ilia Evdokimov
<[email protected]> wrote:
> You're right—floor shouldn't be used since it behaves differently across
> platforms, as Tom also pointed out earlier. I like the idea of using %,
> but since the compiler doesn't allow this operation with double, we need
> to cast it to int64. I checked how nloops and ntuples are modified -
> they are only incremented by 1. So that the casting might be safe. If I
> missed anything or if there's a reason why this casting wouldn't be
> safe, please let me know.

What I've been advocating for is just:

if (nloops > 1)

Instead of:

if (nloops > 1 && rows_is_fractonal)

I don't think it's really safe to just cast a double back to int64. In
practice, the number of tuples should never be large enough to
overflow int64, but if it did, this result would be nonsense. Also, if
the double ever lost precision, the result would be nonsense. If we
want to have an exact count of tuples, we ought to change ntuples and
ntuples2 to be uint64. But I don't think we should do that in this
patch, because that adds a whole bunch of new problems to worry about
and might cause us to get nothing committed. Instead, I think we
should just always show two decimal digits if there's more than one
loop.

That's simpler than what the patch currently does and avoids this
problem. Perhaps it's objectionable for some other reason, but if so,
can somebody please spell out what that reason is so we can talk about
it?

-- 
Robert Haas
EDB: http://www.enterprisedb.com






view thread (38+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: explain analyze rows=%.0f
  In-Reply-To: <CA+TgmobVWYnYDj1HZ--iqeKx+n7zTgtYCar+YfZ0V6wiX-t0Mg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox