Thread: Constant Splitting/Refactoring

Constant Splitting/Refactoring

From
David Christensen
Date:
As part of the reserved page space/page features[1] work, there is a need to convert some of the existing constants into variables, since the size of those will no longer be fixed at compile time.  At FOSDEM, there was some interest expressed about seeing what a cleanup patch like that would look like.

Enclosed is a series of cleanup patches for HEAD.  This splits each of the 4 constants that care about page size into Cluster-specific and -Limit variants, the first intended to become a variable in time, and the second being the maximum value such a variable may take (largely used for static allocations).

Since these patches define these symbols to have the same values they previously had, there are no changes in functionality.  These were largely mechanical changes, and as such we should perhaps consider making the same changes to back-branches to make it so context lines and the like would be the same, simplifying maintainer's efforts when applying code in back branches that touch similar areas.

The script I have to make these changes is simple, and could be run against the back branches with only the comments surrounding Calc() pieces needing to be adjusted once.

These were based on HEAD as a few minutes ago, 902900b308f, and pass all tests.
Thanks,

David

Attachment

Re: Constant Splitting/Refactoring

From
David Christensen
Date:
This should target PG 18, but that status is not available in the CF app yet, so just making a note here.

Re: Constant Splitting/Refactoring

From
David Christensen
Date:
Here is a version 2 of this patch, rebased atop 97d85be365.

As before, this is a cleanup/prerequisite patch series for the page
features/reserved page size patches[1].  (Said patch series is going
to be updated shortly.)

This splits each of the 4 constants that care about page size into
Cluster-specific and -Limit variants, the first intended to become a
variable in time, and the second being the maximum value such a
variable may take (largely used for static
allocations).

Since these patches define these symbols to have the same values they
previously had, there are no changes in functionality.  These were
largely mechanical changes, and as such we should perhaps consider
making the same changes to back-branches to make it so context lines
and the like
would be the same, simplifying maintainer's efforts when applying code
in back branches that touch similar areas.

The script I have to make these changes is simple, and could be run
against the back branches with only the comments surrounding Calc()
pieces needing
to be adjusted once.

Thanks,

David

[1] https://commitfest.postgresql.org/47/3986/

Attachment

Re: Constant Splitting/Refactoring

From
Kirill Reshke
Date:
On Wed, 13 Mar 2024 at 20:42, David Christensen
<david.christensen@crunchydata.com> wrote:
>
> Here is a version 2 of this patch, rebased atop 97d85be365.
>
> As before, this is a cleanup/prerequisite patch series for the page
> features/reserved page size patches[1].  (Said patch series is going
> to be updated shortly.)
>
> This splits each of the 4 constants that care about page size into
> Cluster-specific and -Limit variants, the first intended to become a
> variable in time, and the second being the maximum value such a
> variable may take (largely used for static
> allocations).
>
> Since these patches define these symbols to have the same values they
> previously had, there are no changes in functionality.  These were
> largely mechanical changes, and as such we should perhaps consider
> making the same changes to back-branches to make it so context lines
> and the like
> would be the same, simplifying maintainer's efforts when applying code
> in back branches that touch similar areas.
>
> The script I have to make these changes is simple, and could be run
> against the back branches with only the comments surrounding Calc()
> pieces needing
> to be adjusted once.
>
> Thanks,
>
> David
>
> [1] https://commitfest.postgresql.org/47/3986/

Hi! Your patchset needs a rebase. Are you going to push it forward?

Also, It is better to have more informative commit messages in patches.
-- 
Best regards,
Kirill Reshke