On 3/5/20 7:14 PM, Magnus Hagander wrote:
> On Thu, Mar 5, 2020 at 3:30 PM Michael Paquier <michael@paquier.xyz> wrote:
>>
>> On Thu, Mar 05, 2020 at 12:51:50PM -0500, David Steele wrote:
>>> Looks like you changed 65535 -> 65536 before commit. I checked the original
>>> patch and it has 65535.
>>
>> That's my fault, thanks. I have been playing with the surroundings
>> while looking at your patch.
>
> :/
>
> That's a pretty dangerous mistake, and moreso because we don't have a
> test around it. We should really find a way to add such a test -- just
> a hardcoded page calculation that should always return the same value
> perhaps?
FWIW, we use static values in our unit tests (which are written in C),
then test against packaged versions of Postgres for integration tests.
When I saw the commit I pulled it in so I could remove instructions for
the manual step to add the cast. So in this case the issue was apparent
really quickly. Normally we only pull in new code from PostgreSQL once
a year.
We think our unit tests against static values may have endianess issues
but we have not verified that one way or the other. Here's what they
look like:
https://github.com/pgbackrest/pgbackrest/blob/e55443c890181ea63a350275447885331c8254e4/test/src/module/postgres/interfaceTest.c#L182
Regards,
--
-David
david@pgmasters.net