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
- v11-0002-Clarify-display-of-rows-as-decimal-fractions.patch (text/x-patch) patch v11-0002
- v11-0001-Clarify-display-of-rows-as-decimal-fractions.patch (text/x-patch) patch v11-0001
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
-
EXPLAIN: Always use two fractional digits for row counts.
- 95dbd827f2ed 18.0 landed
-
Adjust EXPLAIN test case to filter out "Actual Rows" values.
- 44cbba9a7f51 18.0 cited
-
Allow EXPLAIN to indicate fractional rows.
- ddb17e387aa2 18.0 landed
-
Fix pgbench performance issue induced by commit af35fe501.
- fb056564ec5b 18.0 cited