Thread

Commits

Retire a few backwards compatibility macros.
- 75680c3d805e 17.0 landed
Retire MemoryContextResetAndDeleteChildren() macro.
- 6a72c42fd5af 17.0 cited
Simplify code by getting rid of SPI_push, SPI_pop, SPI_restore_connection.
- 1833f1a1c3b0 10.0 cited
tuplestore_donestoring() isn't needed anymore, but provide a no-op
- dd04e958c8b0 7.4.1 cited

Do away with a few backwards compatibility macros

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2023-11-16T13:41:41Z

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

Re: Do away with a few backwards compatibility macros

Nathan Bossart <nathandbossart@gmail.com> — 2023-11-16T15:46:22Z

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

Re: Do away with a few backwards compatibility macros

Nathan Bossart <nathandbossart@gmail.com> — 2023-11-21T04:58:40Z

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

Re: Do away with a few backwards compatibility macros

Tom Lane <tgl@sss.pgh.pa.us> — 2023-11-21T05:05:36Z

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

Re: Do away with a few backwards compatibility macros

Nathan Bossart <nathandbossart@gmail.com> — 2023-11-21T15:52:03Z

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

Re: Do away with a few backwards compatibility macros

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2023-11-27T10:59:18Z

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

Re: Do away with a few backwards compatibility macros

Nathan Bossart <nathandbossart@gmail.com> — 2023-11-27T19:14:39Z

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