| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz> |
| Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, pgsql-hackers(at)postgresql(dot)org, euler(at)eulerto(dot)com, andres(at)anarazel(dot)de |
| Subject: | Re: split tablecmds.c |
| Date: | 2025-12-02 08:01:23 |
| Message-ID: | CA+HiwqHVhYQ0qXxUkQ_Hfwg5ZL=asQUaa75-y+7FaEqYiB_Zog@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Dec 2, 2025 at 9:04 AM Michael Paquier <michael(at)paquier(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Antonin Houska | 2025-12-02 08:07:54 | Re: Index functions and INDEX_CREATE_SKIP_BUILD |
| Previous Message | Heikki Linnakangas | 2025-12-02 08:01:06 | Re: Buffer locking is special (hints, checksums, AIO writes) |