Re: explain analyze rows=%.0f

Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>

From: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Matheus Alcantara <matheusssilv97@gmail.com>, Guillaume Lelarge <guillaume@lelarge.info>, Daniel Gustafsson <daniel@yesql.se>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Peter Geoghegan <pg@bowt.ie>, vignesh C <vignesh21@gmail.com>, "David G. Johnston" <david.g.johnston@gmail.com>, Alena Rybakina <a.rybakina@postgrespro.ru>, Andrei Lepikhov <lepihov@gmail.com>, pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2025-02-17T18:53:16Z
Lists: pgsql-hackers

Attachments

On 17.02.2025 17:19, Robert Haas wrote:
> On Mon, Feb 17, 2025 at 3:08 AM Ilia Evdokimov
> <ilya.evdokimov@tantorlabs.com> 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?
>

I have no objections. I agree if we are going to transition from double 
to int64, it should be done properly, but definitely not in current thread.

As for documentation changes, I'll keep t1.unique > t2.unique. If we use 
equality instead, we need to explicitly show rows=1.00, which would 
likely raise too many questions from users what it means.

I attached patches v11 with changes.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.

Commits

  1. EXPLAIN: Always use two fractional digits for row counts.

  2. Adjust EXPLAIN test case to filter out "Actual Rows" values.

  3. Allow EXPLAIN to indicate fractional rows.

  4. Fix pgbench performance issue induced by commit af35fe501.