Thread: Do away with a few backwards compatibility macros
Hi, After a recent commit 6a72c42f (a related discussion [1]) which removed MemoryContextResetAndDeleteChildren(), I think there are a couple of other backward compatibility macros out there that can be removed. These macros are tuplestore_donestoring() which was introduced by commit dd04e95 21 years ago and SPI_push() and friends which were made no-ops macros by commit 1833f1a 7 years ago. Debian code search shows very minimal usages of these macros. Here's a patch attached to remove them. Thoughts? [1] https://www.postgresql.org/message-id/20231114175953.GD2062604%40nathanxps13 -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Attachment
On Thu, Nov 16, 2023 at 07:11:41PM +0530, Bharath Rupireddy wrote: > After a recent commit 6a72c42f (a related discussion [1]) which > removed MemoryContextResetAndDeleteChildren(), I think there are a > couple of other backward compatibility macros out there that can be > removed. These macros are tuplestore_donestoring() which was > introduced by commit dd04e95 21 years ago and SPI_push() and friends > which were made no-ops macros by commit 1833f1a 7 years ago. Debian > code search shows very minimal usages of these macros. Here's a patch > attached to remove them. I'm fine with this because all of these macros are no-ops for all supported versions of Postgres. Even if an extension is using them today, you'll get the same behavior as before if you remove the uses and rebuild against v12-v16. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Thu, Nov 16, 2023 at 09:46:22AM -0600, Nathan Bossart wrote: > On Thu, Nov 16, 2023 at 07:11:41PM +0530, Bharath Rupireddy wrote: >> After a recent commit 6a72c42f (a related discussion [1]) which >> removed MemoryContextResetAndDeleteChildren(), I think there are a >> couple of other backward compatibility macros out there that can be >> removed. These macros are tuplestore_donestoring() which was >> introduced by commit dd04e95 21 years ago and SPI_push() and friends >> which were made no-ops macros by commit 1833f1a 7 years ago. Debian >> code search shows very minimal usages of these macros. Here's a patch >> attached to remove them. > > I'm fine with this because all of these macros are no-ops for all supported > versions of Postgres. Even if an extension is using them today, you'll get > the same behavior as before if you remove the uses and rebuild against > v12-v16. Barring objections, I'll plan on committing this in the next week or so. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes:
> On Thu, Nov 16, 2023 at 09:46:22AM -0600, Nathan Bossart wrote:
>> I'm fine with this because all of these macros are no-ops for all supported
>> versions of Postgres. Even if an extension is using them today, you'll get
>> the same behavior as before if you remove the uses and rebuild against
>> v12-v16.
> Barring objections, I'll plan on committing this in the next week or so.
No objection here, but should we try to establish some sort of project
policy around this sort of change (ie, removal of backwards-compatibility
support)? "Once it no longer matters for any supported version" sounds
about right to me, but maybe somebody has an argument for thinking about
it differently.
regards, tom lane
On Tue, Nov 21, 2023 at 12:05:36AM -0500, Tom Lane wrote: > No objection here, but should we try to establish some sort of project > policy around this sort of change (ie, removal of backwards-compatibility > support)? "Once it no longer matters for any supported version" sounds > about right to me, but maybe somebody has an argument for thinking about > it differently. That seems reasonable to me. I don't think we need to mandate that backwards-compatibility support be removed as soon as it is eligible, but it can be considered fair game at that point. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Tue, Nov 21, 2023 at 9:22 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Tue, Nov 21, 2023 at 12:05:36AM -0500, Tom Lane wrote: > > No objection here, but should we try to establish some sort of project > > policy around this sort of change (ie, removal of backwards-compatibility > > support)? "Once it no longer matters for any supported version" sounds > > about right to me, but maybe somebody has an argument for thinking about > > it differently. > > That seems reasonable to me. I don't think we need to mandate that > backwards-compatibility support be removed as soon as it is eligible, but > it can be considered fair game at that point. I think it's easy to miss/enforce a documented policy. IMV, moving towards pg_attribute_deprecated as Alvaro Herrera said in the other thread https://www.postgresql.org/message-id/202311141920.edtj56saukiv%40alvherre.pgsql can help. Authors then can declare the variables and functions as deprecated so that the code compilation with -Wno-deprecated-declarations can help track all such deprecated code. Having said that, I'm all +1 if the v1 patch proposed in this thread gets in. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On Mon, Nov 27, 2023 at 04:29:18PM +0530, Bharath Rupireddy wrote: > I think it's easy to miss/enforce a documented policy. IMV, moving > towards pg_attribute_deprecated as Alvaro Herrera said in the other > thread https://www.postgresql.org/message-id/202311141920.edtj56saukiv%40alvherre.pgsql > can help. Authors then can declare the variables and functions as > deprecated so that the code compilation with > -Wno-deprecated-declarations can help track all such deprecated code. I'm +1 for adding pg_attribute_deprecated once we have something to use it for. > Having said that, I'm all +1 if the v1 patch proposed in this thread gets in. Committed. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com