Message-ID: <8b633f12-c06f-49ce-827e-ffe23b6c0dc1@iki.fi>
Date: Thu, 14 Nov 2024 11:54:29 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: On non-Windows, hard depend on uselocale(3)
To: Thomas Munro <thomas.munro@gmail.com>,
 Peter Eisentraut <peter@eisentraut.org>
Cc: Tristan Partin <tristan@neon.tech>, Tom Lane <tgl@sss.pgh.pa.us>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <CWZBBRR6YA8D.8EHMDRGLCKCD@neon.tech>
 <CA+hUKGKu7H7iPNQsZ5FcPkZVbgXc5iTBgUjdpM2EHP4q=Tz42Q@mail.gmail.com>
 <12842.1700083028@sss.pgh.pa.us>
 <CA+hUKGJ4s6uDK_KYK-2q=2aWztgvTHYqGeSzugaTCEs6_3XqAg@mail.gmail.com>
 <98824.1700089582@sss.pgh.pa.us>
 <CA+hUKGLLb1MiqAUwtSb6gQsqm8wgAh+7UEE-6P1tQxjhHCFmrw@mail.gmail.com>
 <664255.1700245108@sss.pgh.pa.us> <810814.1700261928@sss.pgh.pa.us>
 <CA+hUKGLy2AeE0a6MHq_KQhJXbOQJ6UHsrUoCwb5OGj+WijAWqw@mail.gmail.com>
 <1155474.1700433384@sss.pgh.pa.us>
 <CA+hUKG+YxfUyUOY7OHUFCgenAgq9znksd=Z+vyb20KHG=2iAnw@mail.gmail.com>
 <1330178.1700498443@sss.pgh.pa.us>
 <CA+hUKG+Yv+ps=nS2T8SS1UDU=iySHSr4sGHYiYGkPTpZx6Ooww@mail.gmail.com>
 <CA+hUKGLs9ccA7pUfphZOuf_fcVq2FAai7Cm0pmbQk0Qcs4VoVQ@mail.gmail.com>
 <CA+hUKGKOuA4vTkFk01RSLqyM0pzNjYm-3h2zAtW48Ffc7FjjNg@mail.gmail.com>
 <CA+hUKG+ue0KC5KSqMuL5zi7uCSuXCLP_B+sZSq5=yS84606ZsA@mail.gmail.com>
 <423491f5-2eaa-454e-8dc9-037d69c1a8f9@eisentraut.org>
 <CA+hUKG+fApBC7Ziq2HP6K-6ymD=AafyvYQq9HGfyRCoBw-PGtQ@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <CA+hUKG+fApBC7Ziq2HP6K-6ymD=AafyvYQq9HGfyRCoBw-PGtQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/8b633f12-c06f-49ce-827e-ffe23b6c0dc1%40iki.fi>
Precedence: bulk

On 14/11/2024 09:48, Thomas Munro wrote:
> On Thu, Aug 29, 2024 at 6:50 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>> The error handling with pg_ensure_c_locale(), that's the sort of thing
>> I'm afraid will be hard to test or even know how it will behave.  And it
>> creates this weird coupling between pgtypeslib and ecpglib that you
>> mentioned earlier.  And if there are other users of PG_C_LOCALE in the
>> future, there will be similar questions about the proper initialization
>> and error handling sequence.
> 
> Ack.  The coupling does become at least less weird (currently it must
> be capable of giving the wrong answers reliably if called directly I
> think, no?) and weaker, but I acknowledge that it's still non-ideal
> that out-of-memory would be handled nicely only if you used ecpg
> first, and subtle misbehaviour would ensure otherwise, and future
> users face the same sort of problem unless they design in a reasonable
> initialisation place that could check pg_ensure_c_locale() nicely.
> Classic retro-fitting problem, though.

Hmm, so should we add calls to pg_ensure_c_locale() in pgtypeslib too, 
before each call to strtod_l()?

Looking at the callers of strtod() in ecpg, all but one of them actually 
readily convert the returned value to integer, with some multiplication 
or division with constants. It would be nice to replace those with a 
function that would avoid going through double in the first place. That 
still leaves one caller in numericvar_to_double() which really wants a 
double, though

Overall, the patch looks good to me.

-- 
Heikki Linnakangas
Neon (https://neon.tech)