Re: missing possibility to use alternative translated month names in to_char function

Hi again,

thanks for the updated patch.
> TAMMON is not implemented, because glibc doesn't provide an
> alternative form for abbreviated month names.
> It is a question if it is better to raise an error, return a non
> alternative name or just ignore this flag. I have not strong
> opinion about this. Inside DCH_to_char the prefix TM is ignored when
> it is not used. So I did the same.
The Locale standard actually mentions abbreviated alternative month
names as
"ab_alt_mon" (see [1]). I tested this by setting your TAMMONTH strftime
call to '%Ob'.
If we set the locale to Russian and call the function for May this
actually returns
an abbreviated version of the month name:

Breakpoint 1, cache_locale_time () at pg_locale.c:772
772           if (strftime_l(bufptr, MAX_L10N_DATA, "%Ob", timeinfo,
locale) <= 0)
(gdb) n
774                     bufptr += MAX_L10N_DATA;
(gdb) print bufptr
$4 = 0x7ffde4554630 "май"

Compared to the TMMON form of May in Russian this actually makes a
difference:

postgres=# set lc_time='ru_RU.UTF8';
SET
postgres=# select to_char('2026-05-01'::date, 'TMMON');
 to_char
---------
 МАЯ
(1 row)

postgres=# select to_char('2026-05-01'::date, 'TAMMONTH');
 to_char
---------
 МАЙ
(1 row)

Again, TAMMONTH uses %Ob here. So I would argue for implementing the
abbreviated
forms too.

>
>     Code Review
>     ===========
>     I am not sure if this condition can be ever true:
>
>     + if (IS_SUFFIX_TM(n->suffix) && IS_SUFFIX_TAM(n->suffix))
>
>     In case this is not possible the subsequent error message will
>     never be
>     thrown and the whole block is dead code.
>
>     However, if there is a reason for this check, it would be nice to
>     have a comment mention it.
>
>
> Yes, I badly expected that prefixes could be mixed. I checked the
> code, and this is not possible, so I removed this check

I think you missed one occurrence in the current patch:

[bernd(at)paco patches]$ cat
v2-0001-introduce-tam-modifier-for-date-timestamp-formatting.patch |
grep -B4 together
+
+ if (IS_SUFFIX_TM(n->suffix) && IS_SUFFIX_TAM(n->suffix))
+         ereturn(escontext,,
+                         (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+                          errmsg("TM and TAM prefixes cannot be used
together")));

>
>     One thing I am not entirely sure about is this line:
>
>     + if (strftime_l(bufptr, MAX_L10N_DATA, "%OB", timeinfo, locale)
> <= 0)
>     +     strftimefail = true;
>
>     As far as I checked %OB is not supported on Windows:
>
> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strftime-wcsftime-strftime-l-wcsftime-l?view=msvc-170
>
>
>     I was, however, not able to confirm this on a Windows machine
>     right now.
>     I might be able to do so on the weekend.
>
>     However, the same problem exists with older glibc versions not
>     supporting
>     %OB. As far as I understand strftime_l this will lead to it
>     returning 0 and
>     therefore setting strftimefail to true always. Since this if
>     statement
>     is evaluated
>     unconditionally I expect this to also influence the behaviour of TM.
>
>
> I am afraid about this case too, and I expect maybe some fallback mode
> there. But because
> I have not a Window machine or some machine with older glibc I decided
> to don't touch it
> for this moment.
>
> glibc older than 2.28 doesn't support %OB. On the second hand the
> result of %B is probably
> equal to expected result %B.

Turns out I was wrong about this one. For one, although %OB is not
supported, Windows
actually does fall back to %B when used:

postgres=# set lc_time='cs-CZ.UTF8';
SET
postgres=# SELECT to_char('2026-08-01'::date, 'TMMONTH');
 to_char
---------
 SRPEN
(1 row)

postgres=# SELECT to_char('2026-08-01'::date, 'TAMMONTH');
 to_char
---------
 SRPEN
(1 row)

The documentation for wcsftime even mentions this behaviour (see [2]).

Furthermore, strftime_l only returns 0 when the result string does not
fit the buffer (or the result is in fact empty, see [3]). The return
value of
a function call with unrecognized format is not defined. glibc seems to
default to returning the format string itself:

PS C:\Users\bernd> docker run --rm -it debian:8 bash -c 'LC_TIME=C date
"+%B"'
August
PS C:\Users\bernd> docker run --rm -it debian:8 bash -c 'LC_TIME=C date
"+%OB"'
%OB

In my opinion this function call is very unlikely to ever return 0
because %OB is not
defined. So I don't see an issue with that code segment after all, as
this patch does
not interfere with TM and the current documentation of this patch
mentions the
platform dependency of this feature. However, one could argue for
checking whether
%OB/%Ob is literally returned and manually falling back to %B/%b to
cover all bases
and make behaviour more consistent.

>
>     Nitpicking & Conclusion
>     =======================
>     I feel like the documentation could explicitly mention the difference
>     between TMMONTH and TAMMONTH (i.e., mentioning genitive and
> nominative
>     like the Locale docs you mentioned do). I also think it would be
>     beneficial to
>     mention that this linguistic detail is specific to certain languages.
>
>
> please, if you can write this part of the doc. My English is not good
> enough to write well about these linguistic details.
> It is a problem primarily for slavic languages - like Czech or Russian
> - but maybe it can be a wide problem.

I attached a revised version of your patch to this message explaining the
difference from a linguistic standpoint. I also fixed the second
listitem not
mentioning the suppression of trailing white space by TAM.

As English is also not my first language, I would be very glad if any
native
speaker reading this could have a look over it. Any feedback is
appreciated!

> Thank you very much for this immediate review
You are most welcome 🙂

Best
Bernd

[1] https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap07.html
[2] https://en.cppreference.com/cpp/chrono/c/wcsftime
[3] https://man7.org/linux/man-pages/man3/strftime.3.html