Re: BUG #18240: Undefined behaviour in cash_mul_flt8() and friends

Jonathan S. Katz <jkatz@postgresql.org>

From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>, David Rowley <dgrowleyml@gmail.com>, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2024-01-13T03:50:25Z
Lists: pgsql-bugs
On 1/12/24 11:33 AM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz@postgresql.org> writes:

>> It sounds like we'd have to tread a bit lightly because of this, even if
>> money is not frequently (or at all) used with btree_gist/gin?
> 
> What'd have to happen is that people would have to upgrade to a
> version of btree_gin/btree_gist that deletes its money support
> before they could pg_upgrade into a core version that lacks money.
> So we'd have to ship that version at least one major release before
> nuking the core support.

Hm -- what about people who skip versions (e.g. 16 => 19)? Would they 
have to stop at 18 first to perform the upgrade? And does this only 
affect people who use btree_gist/gin for moeny, or all btree_gist/gin users?

> I think the shortest timeline we could possibly do this on is:
> 
> v17: label money as deprecated and due for removal in the SGML docs

We had apparently deprecation notice as late as 8.2[1], but this warning 
is missing in the docs :(

> v18: remove support in btree_gin/btree_gist (and any other affected
> extensions)

If the btree_gist/gin issue only affects people who use it in 
combination with money, I'd at least think we consider this for v17, and 
make clear that a users with money have to take action before upgrading.

If we stick with just adding the deprecation notice in the v17 docs, I'd 
also suggest we emit a log warning when loading the catalog if the user 
has an active use of a "money" type in a table. (I understand that may a 
pain to do, but at least wanted to suggested it).

> v19: remove it from core
> 
> Note that in v18, people could still use money even in
> btree_gin/btree_gist, just by installing a non-default extension
> version.  So their C code for money would have to stay.

Yeah, but for what we support directly in PostgreSQL, we will have made 
best effort. If someone _really_ wants to do something with "money" in 
the way you describe above, then they're committed to it.

Thanks,

Jonathan

[1] https://www.postgresql.org/docs/8.2/datatype-money.html

Commits

  1. Add overflow checks to money type.

  2. Reduce dependency to money data type in main regression test suite