On Mon, Feb 17, 2025 at 3:08 AM Ilia Evdokimov <ilya.evdoki...@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? -- Robert Haas EDB: http://www.enterprisedb.com
