Thread: Drop --disable-floatN-byval configure options?
Per the discussion at https://www.postgresql.org/message-id/flat/54dfd2022c205eda9aa35b88923f027a%40postgrespro.ru it's become evident that use of --disable-float8-byval breaks at least one current regression test case, because it results in a large change in hash table size for float8 hash keys. While we could hack around that somehow, I think it would be a poor use of development effort. I have a modest substitute proposal: let's just drop the --disable-float4-byval and --disable-float8-byval configure options as of v11. Those don't have any impact on on-disk storage. As best I can recall, the whole argument for having them was to allow building backends that retained backwards compatibility with old version-0 C functions that might expect pass- by-reference behavior for these types. Now that we've dropped version-0 C function support, there's no point in that; any version-1 function that is written per conventions won't notice the difference. So the extra complication isn't buying useful flexibility, just more cases that we'd have to test if we want to claim that we're supporting these options in any meaningful way. We would need to double-check that pg_upgrade allows converting from byref to byval cases, but offhand I see no check for that in its code, so I think it will just work. (You might be wondering why these flags are reflected in pg_control, if there's no impact on on-disk storage. The reason is that the backend's compiled-in assumptions have to match what pg_type.typbyval says, so pg_control memorializes what initdb put into the catalogs. But we should be able to cope with changing that during a pg_upgrade cycle, just like other catalog content changes.) regards, tom lane
On Wed, Feb 21, 2018 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Per the discussion at > https://www.postgresql.org/message-id/flat/54dfd2022c205eda9aa35b88923f027a%40postgrespro.ru > it's become evident that use of --disable-float8-byval breaks at least one > current regression test case, because it results in a large change in hash > table size for float8 hash keys. While we could hack around that somehow, > I think it would be a poor use of development effort. I have a modest > substitute proposal: let's just drop the --disable-float4-byval and > --disable-float8-byval configure options as of v11. Those don't have any > impact on on-disk storage. USE_FLOAT4_BYVAL seems completely pointless to me, but don't we need USE_FLOAT8_BYVAL on machines where Datum is only 4 bytes wide? If we've got to support that case on such machines, I'm not sure I'm in favor of removing the option to have that behavior on other machines. Being able to test such things without digging up a 32-bit machine is useful. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Feb 21, 2018 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I have a modest
>> substitute proposal: let's just drop the --disable-float4-byval and
>> --disable-float8-byval configure options as of v11. Those don't have any
>> impact on on-disk storage.
> USE_FLOAT4_BYVAL seems completely pointless to me, but don't we need
> USE_FLOAT8_BYVAL on machines where Datum is only 4 bytes wide?
Yes. The point is to have one configuration for 32-bit machines and one
for 64-bit, not three possible configurations on 32-bit and four on
64-bit.
I don't actually envision changing the C code much at all; we might want
to resurrect the old code at some point. I just want to reduce the number
of supported configurations.
> Being able to test such things without digging up a 32-bit machine is
> useful.
Agreed. It'd still be possible to manually force this, it just wouldn't
be a documented/supported configuration.
regards, tom lane
On 2/21/18 12:17, Tom Lane wrote: > I don't actually envision changing the C code much at all; we might want > to resurrect the old code at some point. I just want to reduce the number > of supported configurations. Yeah, it can exist like EXEC_BACKEND, as a way to manually simulate a different architecture for testing. But we don't need it as an option exposed to users. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services