Thread

  1. Re: split tablecmds.c

    amit <amitlangote09@gmail.com> — 2025-12-02T08:01:23Z

    Hi,
    
    On Tue, Dec 2, 2025 at 9:04 AM Michael Paquier <michael@paquier.xyz> wrote:
    > On Mon, Dec 01, 2025 at 04:43:37PM -0600, Nathan Bossart wrote:
    > > I tried to move the partitioning-related code to a new file, and it wasn't
    > > too bad.  Note that there are a couple of internal-to-tablecmds.c things
    > > that need to be exported.  Besides that, the attached patch is still pretty
    > > rough, and I'm not sure I correctly placed the line in the sand when
    > > determining what stays and what goes, but this at least shows the general
    > > shape of what's needed.  (BTW git was generating an atrocious diff for
    > > tablecmds.c.  You might need to set the diff algorithm to "minimal" if you
    > > are similarly affected.)
    
    +1 to splitting tablecmds.c at long last.
    
    (I suppose I or someone could’ve proposed that back in Dec 2016 :-).
    We did create src/backend/partitioning in v11 to move code from then
    big catalog/partition.c, but this one has stayed untouched since then.
    Better late than never.)
    
    > Moving all the partition-specific code into a different file makes
    > sense here.  Is partcmds.c as name the best fit though?  Perhaps a
    > tablecmds_partition.c, with other files named tablecmds_popo.c to
    > indicate the sub-systems formerly in tablecmds.c?
    
    As Andres mentioned, it’s good to avoid slicing too granularly, but I
    also thought of the name tablecmds_partition.c as soon as I saw “split
    tablecmds.c” and “partitioning code.” That seems a reasonable first
    cut.
    
    > >  src/backend/commands/Makefile         |    1 +
    > >  src/backend/commands/meson.build      |    1 +
    > >  src/backend/commands/partcmds.c       | 3377 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
    > >  src/backend/commands/tablecmds.c      | 3456 +--------------------------------------------------------------------------------------------
    > >  src/backend/partitioning/partbounds.c |    1 +
    > >  src/include/commands/partcmds.h       |   53 ++
    > >  src/include/commands/tablecmds.h      |  134 +++-
    > >  7 files changed, 3575 insertions(+), 3448 deletions(-)
    >
    > The new contents of tablecmds.h don't have any strong dependency with
    > tablecmds.h, so perhaps having the "internal" structures like the ones
    > you are moving here into a new tablecmds_internal.h would be cleaner?
    
    +1 to introducing tablecmds_internal.h as well.
    
    > Another sub-area of tablecmds.c that could be split is I think the
    > rewrite logic.  It has a lot of its own perks that become harder to
    > figure out the more tablecmds.c gets bloated.
    
    +1 on splitting out the rewrite logic separately too.
    
    -- 
    Thanks, Amit Langote