Thread: [PATCH] Remove workarounds to format [u]int64's

Hi hackers,

I learned from Tom [1] that we can simplify the code like:

```
char buff[32];
snprintf(buf, sizeof(buf), INT64_FORMAT, ...)
ereport(WARNING, (errmsg("%s ...", buf)));
```

... and rely on %lld/%llu now as long as we explicitly cast the
argument to long long int / unsigned long long. This was previously
addressed in 6a1cd8b9 and d914eb34, but I see more places where we
still use an old approach.

Suggested patch fixes this. Tested locally - no warnings; passes all the tests.

[1] https://www.postgresql.org/message-id/771048.1647528068%40sss.pgh.pa.us

-- 
Best regards,
Aleksander Alekseev

Hi Pavel,

> Probably you can do (long long) instead of (long long int). It is shorter and this is used elsewhere in the code.

Thanks! Here is the updated patch. I also added Reviewed-by: and
Discussion: to the commit message.

-- 
Best regards,
Aleksander Alekseev

On Mon, 21 Mar 2022 at 17:23, Aleksander Alekseev <aleksander@timescale.com> wrote:
> Hi Pavel,
>
>> Probably you can do (long long) instead of (long long int). It is shorter and this is used elsewhere in the code.
>
> Thanks! Here is the updated patch. I also added Reviewed-by: and
> Discussion: to the commit message.

Hi,

After apply the patch, I found pg_checksums.c also has the similar code.

In progress_report(), I'm not sure we can do this replace for this code.

    snprintf(total_size_str, sizeof(total_size_str), INT64_FORMAT,
             total_size / (1024 * 1024));
    snprintf(current_size_str, sizeof(current_size_str), INT64_FORMAT,
             current_size / (1024 * 1024));

    fprintf(stderr, _("%*s/%s MB (%d%%) computed"),
            (int) strlen(current_size_str), current_size_str, total_size_str,
            percent);

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Hi Japin,

> After apply the patch, I found pg_checksums.c also has the similar code.

Thanks for noticing it.

> In progress_report(), I'm not sure we can do this replace for this code.

I added the corresponding change as a separate commit so it can be
easily reverted if necessary.

Here is a complete patchset with some additional changes by me.

-- 
Best regards,
Aleksander Alekseev

Aleksander Alekseev <aleksander@timescale.com> writes:
>> As Tom said in [1], we don't need to touch the *.po files, since those
>> files are managed by the translation team.

> True, but I figured that simplifying the work of the translation team would
> not harm either.

It would not simplify things for them at all, just mess it up.
The master copies of the .po files are kept in a different repo.
Also, I believe that extraction of new message strings is automated
already.

https://www.postgresql.org/docs/devel/nls.html

https://wiki.postgresql.org/wiki/NLS

            regards, tom lane

Hi Tom,

> It would not simplify things for them at all, just mess it up.
> The master copies of the .po files are kept in a different repo.
> Also, I believe that extraction of new message strings is automated
> already.

Got it, thanks. Here is the corrected patch. It includes all the
changes by me and Japin, and doesn't touch PO files.

-- 
Best regards,
Aleksander Alekseev

Aleksander Alekseev <aleksander@timescale.com> writes:
> Got it, thanks. Here is the corrected patch. It includes all the
> changes by me and Japin, and doesn't touch PO files.

Pushed.  I removed now-unnecessary braces, reflowed some lines
as suggested by Pavel, and pgindent'ed (which insisted on adding
spaces after the casts, as is project style).

            regards, tom lane

On 21.03.22 15:37, Aleksander Alekseev wrote:
>> It would not simplify things for them at all, just mess it up.
>> The master copies of the .po files are kept in a different repo.
>> Also, I believe that extraction of new message strings is automated
>> already.
> 
> Got it, thanks. Here is the corrected patch. It includes all the
> changes by me and Japin, and doesn't touch PO files.

I think in some cases we can make this even simpler (and cast-free) by 
changing the underlying variable to be long long instead of int64. 
Especially in cases where the whole point of the variable is to be some 
counter that ends up being printed, there isn't a need to use int64 in 
the first place.  See attached patch for examples.