Horton

Thread

Commits

  1. Update comment related to C99

  2. Fix pg_isblank()

  3. Replace internal C function pg_hypot() by standard hypot()

  4. Remove obsolete comment

  5. Report timezone offset in pg_dump/pg_dumpall

  1. more C99 cleanup

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-21T13:50:03Z

    I have been hunting down the last few pieces of code that made some 
obsolete claims about not being able to use C99 yet or needing to be 
compatible with pre-C99 or something like that.  I found two obsolete 
comments and two places where we could now actually use the C99 
functionality that the comment was anticipating.

At least according to my grepping, all the remaining mentions of "C99" 
are now up to date and relevant.

  2. Re: more C99 cleanup

    Thomas Munro <thomas.munro@gmail.com> — 2025-11-21T14:03:15Z

    On Sat, Nov 22, 2025 at 2:50 AM Peter Eisentraut <peter@eisentraut.org> wrote:
+ * %z doesn't actually act differently from %Z on Windows.

Probably obsolete too, at least according to:

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strftime-wcsftime-strftime-l-wcsftime-l?view=msvc-170

  3. Re: more C99 cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-21T15:35:16Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> I have been hunting down the last few pieces of code that made some 
> obsolete claims about not being able to use C99 yet or needing to be 
> compatible with pre-C99 or something like that.  I found two obsolete 
> comments and two places where we could now actually use the C99 
> functionality that the comment was anticipating.

Two comments:

* In 0001,

- * This implementation conforms to IEEE Std 1003.1 and GLIBC, in that the
- * case of hypot(inf,nan) results in INF, and not NAN.

I have a distinct recollection that this comment exists because we
found that some platforms had a hypot() that got that edge case wrong.
I don't object to proceeding on the assumption that they all conform
to spec by now, but please make sure there's at least one regression
test that will expose the problem if someplace doesn't.  (A quick check
would be to hot-wire pg_hypot to do the wrong thing and see if any
existing test falls over.  I think there is one, but let's verify.)

* In 0003, we can't be very sure what "isblank((unsigned char) c)"
will do with non-ASCII input data, except that it could very
easily do insane things with fragments of a multibyte character.
I recommend keeping pg_isblank but redefining it along the lines of

	bool
	pg_isblank(unsigned char c)
	{
	    return (!IS_HIGHBIT_SET(c) && isblank(c));
	}

to ensure the previous treatment of non-ASCII data.
(It could be made static in hba.c, perhaps)

			regards, tom lane

  4. Re: more C99 cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-21T16:10:47Z

    I wrote:
> I have a distinct recollection that this comment exists because we
> found that some platforms had a hypot() that got that edge case wrong.
> I don't object to proceeding on the assumption that they all conform
> to spec by now, but please make sure there's at least one regression
> test that will expose the problem if someplace doesn't.  (A quick check
> would be to hot-wire pg_hypot to do the wrong thing and see if any
> existing test falls over.  I think there is one, but let's verify.)

Ah, there are several.  It's not totally obvious perhaps where the
cause is.  I'll attach the diffs just for the archives' sake.

			regards, tom lane

  5. Re: more C99 cleanup

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-25T05:45:05Z

    On 21.11.25 15:03, Thomas Munro wrote:
> On Sat, Nov 22, 2025 at 2:50 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> + * %z doesn't actually act differently from %Z on Windows.
> 
> Probably obsolete too, at least according to:
> 
> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strftime-wcsftime-strftime-l-wcsftime-l?view=msvc-170

Right, we could got back to using %z on all platforms, as originally 
attempted by commit ad5d46a4494.

  6. Re: more C99 cleanup

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-25T05:46:53Z

    On 21.11.25 17:10, Tom Lane wrote:
> I wrote:
>> I have a distinct recollection that this comment exists because we
>> found that some platforms had a hypot() that got that edge case wrong.
>> I don't object to proceeding on the assumption that they all conform
>> to spec by now, but please make sure there's at least one regression
>> test that will expose the problem if someplace doesn't.  (A quick check
>> would be to hot-wire pg_hypot to do the wrong thing and see if any
>> existing test falls over.  I think there is one, but let's verify.)
> 
> Ah, there are several.  It's not totally obvious perhaps where the
> cause is.  I'll attach the diffs just for the archives' sake.

For clarification, does what you are showing mean that the regression 
tests have enough coverage of the hypot() edge cases?

  7. Re: more C99 cleanup

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-25T05:53:55Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> On 21.11.25 17:10, Tom Lane wrote:
>> Ah, there are several.  It's not totally obvious perhaps where the
>> cause is.  I'll attach the diffs just for the archives' sake.

> For clarification, does what you are showing mean that the regression 
> tests have enough coverage of the hypot() edge cases?

We are definitely covering it.  It's just that the present tests
fail in a way that wouldn't scream "hypot is broken" to some
future hacker dealing with such a failure.  Not sure if that's
worth worrying about; but I'd lean to not worrying unless you
commit and we see such failures in the buildfarm.

			regards, tom lane

  8. Re: more C99 cleanup

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-28T07:40:50Z

    On 21.11.25 16:35, Tom Lane wrote:
> * In 0003, we can't be very sure what "isblank((unsigned char) c)"
> will do with non-ASCII input data, except that it could very
> easily do insane things with fragments of a multibyte character.
> I recommend keeping pg_isblank but redefining it along the lines of
> 
> 	bool
> 	pg_isblank(unsigned char c)
> 	{
> 	    return (!IS_HIGHBIT_SET(c) && isblank(c));
> 	}
> 
> to ensure the previous treatment of non-ASCII data.
> (It could be made static in hba.c, perhaps)

Ok, done that way.

  9. Re: more C99 cleanup

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-02T07:27:50Z

    On 25.11.25 06:45, Peter Eisentraut wrote:
> On 21.11.25 15:03, Thomas Munro wrote:
>> On Sat, Nov 22, 2025 at 2:50 AM Peter Eisentraut 
>> <peter@eisentraut.org> wrote:
>> + * %z doesn't actually act differently from %Z on Windows.
>>
>> Probably obsolete too, at least according to:
>>
>> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/ 
>> strftime-wcsftime-strftime-l-wcsftime-l?view=msvc-170
> 
> Right, we could got back to using %z on all platforms, as originally 
> attempted by commit ad5d46a4494.

I decided not to go further into researching and/or changing this, so I 
just updated the comment to indicate that the previously stated problems 
might now be obsolete.

With that, all the patches mentioned in this thread have been committed.