On Fri, Apr 1, 2022 at 8:06 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Joseph Koshakow <kosh...@gmail.com> writes: > > * The existing code for rounding had a lot of int to double > > casting and vice versa. I *think* that doubles are able to completely > > represent the range of ints. However doubles are not able to represent > > the full range of int64. After making the change I started noticing > > a lot of lossy behavior. One thought I had was to change the doubles > > to long doubles, but I wasn't able to figure out if long doubles could > > completely represent the range of int64. Especially since their size > > varies depending on the architecture. Does anyone know the answer to > > this? > > I agree that relying on long double is not a great plan. However, > I'm not seeing where there's a problem. AFAICS the revised code > only uses doubles to represent fractions from the input, ie if you > write "123.456 hours" then the ".456" is carried around for awhile > as a float. This does not seem likely to pose any real-world > problem; do you have a counterexample?
Yeah, you're correct, I don't think there is any problem with just using double. I don't exactly remember why I thought long double was necessary in the revised code. I probably just confused myself because it would have been necessary with the old rounding code, but not the revised code. > Anyway, I've spent today reviewing the code and cleaning up things > I didn't like, and attached is a v10. Thanks so much for the review and updates! > I think the patch can be salvaged, though. I like the concept > of converting all the sub-day fields to microseconds immediately, > because it avoids a host of issues, so I don't want to give that up. > What I'm going to look into is detecting the sign-adjustment-needed > case up front (which is easy enough, since it's looking at the > input data not the conversion results) and then forcing the > individual field values negative before we accumulate them into > the pg_itm_in struct. This sounds like a very reasonable and achievable approach to me. - Joe Koshakow