Thread: Commands/ directory reorganisation
Dear all, Patch attached in line with my previous statement on -hackers about reorganisation of files in the commands directory. (Note that define.c has been kept, holding some of the common support code for define routines.) I have checked this and it compiles and passes regression tests. It is just straightforward moving of code from one place to another. I will have to be out until ~1700 UTC Monday -at which point I will be happy to answer any questions which arise. Regards John Notes to Committer ------------------ The following files are removed in this patch: src/backend/commands/command.c src/backend/commands/creatinh.c src/backend/commands/remove.c src/backend/commands/rename.c src/include/commands/command.h src/include/commands/creatinh.h src/include/commands/rename.h The following files are added: src/backend/commands/aggregatecmds.c src/backend/commands/domaincmds.c src/backend/commands/functioncmds.c src/backend/commands/lockcmds.c src/backend/commands/operatorcmds.c src/backend/commands/portalcmds.c src/backend/commands/schemacmds.c src/backend/commands/tablecmds.c src/backend/commands/typecmds.c src/include/commands/lockcmds.h src/include/commands/portalcmds.h src/include/commands/schemacmds.h src/include/commands/tablecmds.h Details for commit message: The contents of command.c, creatinh.c, define.c, remove.c and rename.c have been divided according to the type of object manipulated -so ALTER TABLE code is in tablecmds.c, aggregate commands in aggregatecmds.c and so on. A few support routines remain in define.c (prototypes in src/include/commands/defrem.h No code has been changed except for includes to reflect the new files. The prototypes for aggregatecmds.c, domaincmds.c, functioncmds.c, operatorcmds.c, and typecmds.c remain in src/include/commands/defrem.h
Attachment
OK, what time timeframe are we in for application of this patch? If I get OK from a few people in the next few hours, do I apply so the code doesn't drift? --------------------------------------------------------------------------- John Gray wrote: > Dear all, > > Patch attached in line with my previous statement on -hackers about > reorganisation of files in the commands directory. (Note that define.c > has been kept, holding some of the common support code for define > routines.) > > I have checked this and it compiles and passes regression tests. It is > just straightforward moving of code from one place to another. > > I will have to be out until ~1700 UTC Monday -at which point I will be > happy to answer any questions which arise. > > Regards > > John > > > Notes to Committer > ------------------ > > The following files are removed in this patch: > > src/backend/commands/command.c > src/backend/commands/creatinh.c > src/backend/commands/remove.c > src/backend/commands/rename.c > src/include/commands/command.h > src/include/commands/creatinh.h > src/include/commands/rename.h > > The following files are added: > > src/backend/commands/aggregatecmds.c > src/backend/commands/domaincmds.c > src/backend/commands/functioncmds.c > src/backend/commands/lockcmds.c > src/backend/commands/operatorcmds.c > src/backend/commands/portalcmds.c > src/backend/commands/schemacmds.c > src/backend/commands/tablecmds.c > src/backend/commands/typecmds.c > > src/include/commands/lockcmds.h > src/include/commands/portalcmds.h > src/include/commands/schemacmds.h > src/include/commands/tablecmds.h > > > Details for commit message: > > The contents of command.c, creatinh.c, define.c, remove.c and rename.c > have been divided according to the type of object manipulated -so ALTER > TABLE code is in tablecmds.c, aggregate commands in aggregatecmds.c and > so on. > > A few support routines remain in define.c (prototypes in > src/include/commands/defrem.h > > No code has been changed except for includes to reflect the new files. > The prototypes for aggregatecmds.c, domaincmds.c, functioncmds.c, > operatorcmds.c, and typecmds.c remain in src/include/commands/defrem.h > > [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> OK, what time timeframe are we in for application of this patch? If I
> get OK from a few people in the next few hours, do I apply so the code
> doesn't drift?
We should apply (or reject) quickly, since as long as the patch is
outstanding it will be holding up other work in backend/commands/.
I know it's in my way for further schema work, for example.
I'll try to look it over this evening; anyone else want to review it?
regards, tom lane
John Gray <jgray@azuli.co.uk> writes:
> Patch attached in line with my previous statement on -hackers about
> reorganisation of files in the commands directory. (Note that define.c
> has been kept, holding some of the common support code for define
> routines.)
This patch looks reasonable to me, with one significant gripe and a
couple of minor ones.
The significant gripe is that I don't think domaincmds.c should exist;
domains are not really different from types and so I think their
commands should be in typecmds.c. Splitting into two files doesn't do
much except eliminate the possibility of sharing code via "static"
subroutines.
I have one or two trivial coding-style issues too, like not wanting to
see "extern" on routine definitions.
If no one has any other objections, I'll fix the things that are
bothering me and check it in.
regards, tom lane
Tom Lane wrote: > John Gray <jgray@azuli.co.uk> writes: > > Patch attached in line with my previous statement on -hackers about > > reorganisation of files in the commands directory. (Note that define.c > > has been kept, holding some of the common support code for define > > routines.) > > This patch looks reasonable to me, with one significant gripe and a > couple of minor ones. > > The significant gripe is that I don't think domaincmds.c should exist; > domains are not really different from types and so I think their > commands should be in typecmds.c. Splitting into two files doesn't do > much except eliminate the possibility of sharing code via "static" > subroutines. > > I have one or two trivial coding-style issues too, like not wanting to > see "extern" on routine definitions. > > If no one has any other objections, I'll fix the things that are > bothering me and check it in. Sounds good to me. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
I'll give it a try sometime today. Chris > -----Original Message----- > From: pgsql-patches-owner@postgresql.org > [mailto:pgsql-patches-owner@postgresql.org]On Behalf Of Tom Lane > Sent: Monday, 15 April 2002 7:57 AM > To: Bruce Momjian > Cc: John Gray; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] Commands/ directory reorganisation > > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > OK, what time timeframe are we in for application of this patch? If I > > get OK from a few people in the next few hours, do I apply so the code > > doesn't drift? > > We should apply (or reject) quickly, since as long as the patch is > outstanding it will be holding up other work in backend/commands/. > I know it's in my way for further schema work, for example. > > I'll try to look it over this evening; anyone else want to review it? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org >
Tom Lane writes: > If no one has any other objections, I'll fix the things that are > bothering me and check it in. Isn't the "cmds" in the file names kind of redundant? -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote: > Tom Lane writes: > > > If no one has any other objections, I'll fix the things that are > > bothering me and check it in. > > Isn't the "cmds" in the file names kind of redundant? Tom wanted it because he file rule.c etc would conflict with a rule.c somewhere else in the tree. Not sure how important that is. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Peter Eisentraut <peter_e@gmx.net> writes:
> Isn't the "cmds" in the file names kind of redundant?
That was at my suggestion, actually. I thought names like "aggregate.c"
were too generic --- too easy to confuse with similarly-named files
elsewhere in the tree. (For example, I regularly get confused by the
existence of analyze.c in both commands/ and parser/.)
regards, tom lane
John Gray <jgray@azuli.co.uk> writes:
> Patch attached in line with my previous statement on -hackers about
> reorganisation of files in the commands directory. (Note that define.c
> has been kept, holding some of the common support code for define
> routines.)
Patch applied.
I believe you also had plans for refactoring the ALTER TABLE routines to
eliminate duplicate coding?
regards, tom lane
On Sun, 14 Apr 2002, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > Isn't the "cmds" in the file names kind of redundant? > > That was at my suggestion, actually. I thought names like "aggregate.c" > were too generic --- too easy to confuse with similarly-named files > elsewhere in the tree. (For example, I regularly get confused by the > existence of analyze.c in both commands/ and parser/.) ya, but shouldn't the fact that its commands/analyze.c and parser/analyze.c eliminate the confusion? :)
On Mon, 2002-04-15 at 06:30, Tom Lane wrote: > John Gray <jgray@azuli.co.uk> writes: > > Patch attached in line with my previous statement on -hackers about > > reorganisation of files in the commands directory. (Note that define.c > > has been kept, holding some of the common support code for define > > routines.) > > Patch applied. > Thanks. > I believe you also had plans for refactoring the ALTER TABLE routines to > eliminate duplicate coding? > I'd had some discussion with Christopher Kings-Lynne on this. He felt (and it seemed a good idea) that we could do that once the file rearrangement was done - I will be looking at that again over the next couple of days. Regards John
Until you open it in an editor with tabs that only shows the filename (on the tab). Sure, once you've selected the file you know what it is -- but not at a quick glance. -- Rod Taylor Your eyes are weary from staring at the CRT. You feel sleepy. Notice how restful it is to watch the cursor blink. Close your eyes. The opinions stated above are yours. You cannot imagine why you ever felt otherwise. ----- Original Message ----- From: "Marc G. Fournier" <scrappy@hub.org> To: "Tom Lane" <tgl@sss.pgh.pa.us> Cc: "Peter Eisentraut" <peter_e@gmx.net>; "John Gray" <jgray@azuli.co.uk>; <pgsql-patches@postgresql.org>; "Bruce Momjian" <pgman@candle.pha.pa.us> Sent: Monday, April 15, 2002 1:42 PM Subject: Re: [PATCHES] Commands/ directory reorganisation > On Sun, 14 Apr 2002, Tom Lane wrote: > > > Peter Eisentraut <peter_e@gmx.net> writes: > > > Isn't the "cmds" in the file names kind of redundant? > > > > That was at my suggestion, actually. I thought names like "aggregate.c" > > were too generic --- too easy to confuse with similarly-named files > > elsewhere in the tree. (For example, I regularly get confused by the > > existence of analyze.c in both commands/ and parser/.) > > ya, but shouldn't the fact that its commands/analyze.c and > parser/analyze.c eliminate the confusion? :) > > > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly >