Re: [PATCH] replace float8 with int in date2isoweek() and date2isoyear()

Sure,

I'm attaching v2 of the patch with "result" renamed to "week".

--

Sergey

On 7/12/25 18:15, Tom Lane wrote:
> =?utf-8?q?=D0=A4=D1=83=D0=BA=D0=B0=D0=BD=D1=87=D0=B8=D0=BA_=D0=A1=D0=B5=D1=80=D0=B3=D0=B5=D0=B9?=
<s.fukanchik@postgrespro.ru>writes:
 
>> Hi PG hackers,
>> I found suspicious use of float8 in date2isoweek() and date2isoyear(). In both
>> cases float8 is only used for storing the value, while the entire calculation
>> on the right happens in integers:
>>   float8 result = (dayn - (day4 - day0)) / 7 + 1;
>> At the end date2isoweek() returns `result' converted back to int:
>>   return (int) result;
>> float8 here is confusing and a bit slow.
> I looked into our git history to try to find out why it's like this.
> The answer seems to be that commit dffd8cac3 created date2isoweek()
> by splitting out pre-existing code that had been in timestamp_part().
> In that context the code had been using a float8 "result" variable
> that was shared with other switch cases, and that variable's type
> was just blindly copied into date2isoweek().  Then 1c757c49f again
> copied-and-pasted while creating date2isoyear().
>
> I agree with getting rid of the unnecessary usage of float8 here,
> but there's another aspect that's bugging me: "result" is a totally
> misleading variable name in date2isoyear(), because it's *not*
> the function's result.  I'm inclined to rename it to "week", and
> then to keep these functions looking as parallel as possible,
> I'd probably do the same in date2isoweek().
>
>> I think there is no need in adding an extra test case here, because
>> date2isoweek and date2isoyear are covered by three regression tests:
> Agreed, the code coverage report shows these are covered.
>
>             regards, tom lane