On Wed, 2024-08-07 at 22:44 +0200, Peter Eisentraut wrote: > But after this patch set, locale cannot be NULL anymore, so the third > branch is obsolete.
... > Second, there are a number of functions in like.c like the above that > take separate arguments like pg_locale_t locale, bool locale_is_c. > Because pg_locale_t now contains the locale_is_c information, these > can > be combined. I believe these patches are correct, but the reasoning is fairly complex: 1. Some MatchText variants are called with 0 for locale. But that's OK because ... 2. A MatchText variant only cares about the locale if MATCH_LOWER(t) is defined, and ... 3. Only one variant, SB_IMatchText() defines MATCH_LOWER(), and ... 4. SB_IMatchText() is called with a non-zero locale. All of these are a bit confusing to follow because it's generated code. #2 is particularly non-obvious, because "locale" is not even an argument of the MATCH_LOWER(t) or GETCHAR(t) macros, it's taken implicitly from the outer scope. I don't think your patches cause this confusion, but is there a way you can clarify some of this along the way? Regards, Jeff Davis