Refactoring of command.c
I thought I should just raise a couple of points -I think I may have
been the person Tom Lane was referring with respect to refactoring
command.c. I am still happy to go through and do this, though I think
that Neil Conway may be active there at present.
1) Division between files.
I imagined that the ALTER TABLE would move to alter.c (but maybe this
should be altertable.c?), the portal support would live in portal.c and
I wasn't sure where LOCK TABLE would go. If we're going to have a
command for showing current locks (I seem to remember that from some
time back, maybe it would go there too?)
2) Permissions checking and inheritance.
Much of the permissions checking and setup on the alter table variants
is the same. In my original patch I'd extracted this into a helper
routine. Also, all the commands which support inheritance do so using a
piece of boilerplate code -I had thought this could become a pair of
macros, either side of whatever action was to be performed on the
descendant.
3) ALTER TABLE CREATE TOAST TABLE
I don't see any need for this in the grammar! AlterTableAddColumn calls
it after a catalogue change, it is called (from ProcessUtility
tcop/utility.c) for CREATE TABLE, and InitPlan calls it for SELECT INTO.
I can't see any useful circumstance in which the command could be
issued, which makes it seem a good candidate for removal from the
grammar.
Any comments on these?
John
John Gray <jgray@azuli.co.uk> writes:
3) ALTER TABLE CREATE TOAST TABLE
I can't see any useful circumstance in which the command could be
issued, which makes it seem a good candidate for removal from the
grammar.
initdb counts as moderately useful ;-)
regards, tom lane
John Gray writes:
I imagined that the ALTER TABLE would move to alter.c (but maybe this
should be altertable.c?),
Put all the table creation and altering code into the same file.
--
Peter Eisentraut peter_e@gmx.net
On Tue, 2002-02-26 at 19:19, Peter Eisentraut wrote:
John Gray writes:
I imagined that the ALTER TABLE would move to alter.c (but maybe this
should be altertable.c?),Put all the table creation and altering code into the same file.
Following this, here is an overview of a possible rearrangement of the
files in the commands/ directory. Those not mentioned I intend to leave
alone. Obviously, I'd also change the include files (and any files
referencing them) to match.
The aim is that each filename will be either a) the name of a command
(explain, analyze, vacuum) or b) a type/class of database object.
(async.c would remain an exception to this)
The new table.c should be amenable to quite a bit of clearing up. I
expect that it might come out at around 9000 lines before that's done,
which is quite long, but it would at least be a well-defined subsystem.
command.c ALTER TABLE -> table.c
portal -> portal.c
LOCK -> lock.c
creatinh.c all code -> table.c
dbcommands.c rename to database.c (see below)
define.c see below
indexcmds.c rename to index.c (see below)
proclang.c case_translate_language_name currently also in define.c
remove.c see below
rename.c update_ri_trigger_args -> trigger.c
renameatt, renamerel -> table.c
define.c and remove.c
These files currently support the "smaller" entities. These could
obviously be split out, so that we have function.c, operator.c,
opclass.c, aggregate.c . This adds more files, but has the merit of
making it obvious where any further future support for these entities
should go. Any feelings about this?
renamed files
I know that "renamed" files in CVS will lose their history (for
tracability we presumably wouldn't rename them in the repository), so I
accept that it may not be a great idea to rename them, but it seems a
little redundant to use commands/dbcommands.c rather than
commands/database.c.
As I'm new to this kind of change, I assume that I'd just submit a
normal context diff for this and rely on it not getting tangled up with
any other patches to these files? Or is this *too* radical a reshuffle?
:-)
John
I know that "renamed" files in CVS will lose their history (for
tracability we presumably wouldn't rename them in the repository), so I
accept that it may not be a great idea to rename them, but it seems a
little redundant to use commands/dbcommands.c rather than
commands/database.c.As I'm new to this kind of change, I assume that I'd just submit a
normal context diff for this and rely on it not getting tangled up with
any other patches to these files? Or is this *too* radical a reshuffle?
:-)
Post the patch and we will try to get it approved quickly for
application. That's about the only clean way I know to do it.
--
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 19026John Gray writes:
dbcommands.c rename to database.c (see below)
indexcmds.c rename to index.c (see below)
Might as well keep these. They don't hurt anyone just because they spell
a little differently.
--
Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote:
John Gray writes:
dbcommands.c rename to database.c (see below)
indexcmds.c rename to index.c (see below)Might as well keep these. They don't hurt anyone just because they spell
a little differently.
I disagree. If we are cleaning, let's clean. *cmd* is redundant. The
contents of the files will be quite different anyway.
--
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 19026Bruce Momjian <pgman@candle.pha.pa.us> writes:
Peter Eisentraut wrote:
John Gray writes:
dbcommands.c rename to database.c (see below)
indexcmds.c rename to index.c (see below)Might as well keep these. They don't hurt anyone just because they spell
a little differently.
I disagree. If we are cleaning, let's clean. *cmd* is redundant. The
contents of the files will be quite different anyway.
But if we rename them, we will lose CVS history for the files (unless we
jump through hoops that typically are not jumped through). I agree with
Peter: do not rename files simply to have slightly simpler file names.
regards, tom lane
pgman wrote:
Peter Eisentraut wrote:
John Gray writes:
dbcommands.c rename to database.c (see below)
indexcmds.c rename to index.c (see below)Might as well keep these. They don't hurt anyone just because they spell
a little differently.I disagree. If we are cleaning, let's clean. *cmd* is redundant. The
contents of the files will be quite different anyway.
Good point about keeping CVS logs. Can't we just move the CVS file to
another name, create an empty file in its place, then delete it. I
thought that would move the history. We have never done it but I would
think there is a way to do it.
--
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 19026Import Notes
Reply to msg id not found: | Resolved by subject fallback
John Gray <jgray@azuli.co.uk> writes:
As I'm new to this kind of change, I assume that I'd just submit a
normal context diff for this and rely on it not getting tangled up with
any other patches to these files? Or is this *too* radical a reshuffle?
As far as mechanics go, the main problem is that you can't expect those
files to hold still while you contemplate what to do with them; there's
probably a commit every day or two that hits at least one.
Your sketch of what-goes-where is a good first cut for discussion. Once
you've learned what you can at this level, I'd suggest preparing a much
more detailed sketch, at the per-routine level, and posting that for
discussion. Once people are happy with that, you can coordinate making
the actual patch and passing it to one of the committers for immediate
application.
Once it does get down to the actual diff, I'd suggest a context diff
plus specific notes to the committer that "this patch adds files a,b,c
and removes files x,y,z". The cvs adds and cvs removes are extra steps,
of course, so it's good to point them out to be sure they're not missed.
regards, tom lane
A simple copy works quite well. Bit of a waste of space but you
preserve the history in both locations.
An empty file will not work. If someone were to check out an older
version it would be broken.
--
Rod Taylor
This message represents the official view of the voices in my head
----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: <pgman@candle.pha.pa.us>
Cc: "Peter Eisentraut" <peter_e@gmx.net>; "John Gray"
<jgray@azuli.co.uk>; <pgsql-hackers@postgresql.org>
Sent: Wednesday, February 27, 2002 12:31 AM
Subject: Re: [HACKERS] Refactoring of command.c
pgman wrote:
Peter Eisentraut wrote:
John Gray writes:
dbcommands.c rename to database.c (see below)
indexcmds.c rename to index.c (see below)Might as well keep these. They don't hurt anyone just because
they spell
a little differently.
I disagree. If we are cleaning, let's clean. *cmd* is redundant.
The
contents of the files will be quite different anyway.
Good point about keeping CVS logs. Can't we just move the CVS file
to
another name, create an empty file in its place, then delete it. I
thought that would move the history. We have never done it but I
would
think there is a way to do it.
-- 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
---------------------------(end of
broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to
majordomo@postgresql.org)
Show quoted text
"Rod Taylor" <rbt@zort.ca> writes:
A simple copy works quite well. Bit of a waste of space but you
preserve the history in both locations.
If you do that, the copied file will appear to CVS to be part of older
versions, no?
regards, tom lane
It will, but I've always been able to make the assumption it won't be
compiled in as the make files always specified exactly which ones to
compile (and would ignore the new 'stray').
Perhaps the best way to approach this would be to ask the FreeBSD
'repo' people what they do -- as they do this kind of stuff quite
frequently.
--
Rod Taylor
This message represents the official view of the voices in my head
----- Original Message -----
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: "Bruce Momjian" <pgman@candle.pha.pa.us>; "Peter Eisentraut"
<peter_e@gmx.net>; "John Gray" <jgray@azuli.co.uk>;
<pgsql-hackers@postgresql.org>
Sent: Wednesday, February 27, 2002 10:13 AM
Subject: Re: [HACKERS] Refactoring of command.c
"Rod Taylor" <rbt@zort.ca> writes:
A simple copy works quite well. Bit of a waste of space but you
preserve the history in both locations.If you do that, the copied file will appear to CVS to be part of
older
Show quoted text
versions, no?
regards, tom lane
Dug the below out of googles cache -- it's what the BSDs do for moving
files in cvs.
What is a repo-copy?
A repo-copy (which is a short form of ``repository copy'') refers to
the direct copying of files within the CVS repository.
Without a repo-copy, if a file needed to be copied or moved to another
place in the repository, the committer would run cvs add to put the
file in its new location, and then cvs rm on the old file if the old
copy was being removed.
The disadvantage of this method is that the history (i.e. the entries
in the CVS logs) of the file would not be copied to the new location.
As the FreeBSD Project considers this history very useful, a
repository copy is often used instead. This is a process where one of
the repository meisters will copy the files directly within the
repository, rather than using the cvs program.
--
Rod Taylor
This message represents the official view of the voices in my head
----- Original Message -----
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: "Bruce Momjian" <pgman@candle.pha.pa.us>; "Peter Eisentraut"
<peter_e@gmx.net>; "John Gray" <jgray@azuli.co.uk>;
<pgsql-hackers@postgresql.org>
Sent: Wednesday, February 27, 2002 10:13 AM
Subject: Re: [HACKERS] Refactoring of command.c
"Rod Taylor" <rbt@zort.ca> writes:
A simple copy works quite well. Bit of a waste of space but you
preserve the history in both locations.If you do that, the copied file will appear to CVS to be part of
older
Show quoted text
versions, no?
regards, tom lane
"Rod Taylor" <rbt@zort.ca> writes:
Dug the below out of googles cache -- it's what the BSDs do for moving
files in cvs.
What is a repo-copy?
A repo-copy (which is a short form of ``repository copy'') refers to
the direct copying of files within the CVS repository.
Yeah, I think that's what we discussed the last time the question came
up.
It seems awfully wrongheaded to me. IMHO, the entire point of a CVS
repository is to store past states of your software, not only the
current state. Destroying the accurate representation of your historical
releases is a poor tradeoff for making it a little easier to find the
log entries for code that's been moved around. What's the point
of having history, if it's not accurate?
regards, tom lane
What is a repo-copy?
A repo-copy (which is a short form of ``repository copy'') refers to
the direct copying of files within the CVS repository.Yeah, I think that's what we discussed the last time the question came
up.It seems awfully wrongheaded to me. IMHO, the entire point of a CVS
repository is to store past states of your software, not only the
current state. Destroying the accurate representation of your historical
releases is a poor tradeoff for making it a little easier to find the
log entries for code that's been moved around. What's the point
of having history, if it's not accurate?
Sounds like it's time to move to using 'arch':
Supports everything that CVS doesn't, including rename events...
BTW - I'm not _seriously_ suggesting this change - but it would be cool,
wouldn't it?
People could start their own local branches which are part of the global
namespace, easily merge them in, etc...
Chris
On Thu, 28 Feb 2002, Christopher Kings-Lynne wrote:
[Shame on Christopher for breaking attributions]
What is a repo-copy?
A repo-copy (which is a short form of ``repository copy'') refers to
the direct copying of files within the CVS repository.Yeah, I think that's what we discussed the last time the question came
up.It seems awfully wrongheaded to me. IMHO, the entire point of a CVS
repository is to store past states of your software, not only the
current state. Destroying the accurate representation of your historical
releases is a poor tradeoff for making it a little easier to find the
log entries for code that's been moved around. What's the point
of having history, if it's not accurate?Sounds like it's time to move to using 'arch':
I see this going down the road of a religious debate, and to prove the
point, I bring up BitKeeper:
Supports everything that CVS doesn't, including rename events...
So does BitKeeper :)
BTW - I'm not _seriously_ suggesting this change - but it would be cool,
wouldn't it?People could start their own local branches which are part of the global
namespace, easily merge them in, etc...
This seems quite pointless for PostgreSQL's development.
C'est la vie.
--
Dominic J. Eidson
"Baruk Khazad! Khazad ai-menu!" - Gimli
-------------------------------------------------------------------------------
http://www.the-infinite.org/ http://www.the-infinite.org/~dominic/
"Dominic J. Eidson" <sauron@the-infinite.org> writes:
On Thu, 28 Feb 2002, Christopher Kings-Lynne wrote:
Sounds like it's time to move to using 'arch':
I see this going down the road of a religious debate, and to prove the
point, I bring up BitKeeper:
Hmm. I'd surely be the last to claim that CVS is the be-all and end-all
of software archiving systems. But is BitKeeper, or arch, or anything
else enough better to justify the pain of switching? This is intended
as an honest question, not flamebait --- I haven't looked closely at
the alternatives.
regards, tom lane
I see this going down the road of a religious debate, and to prove the
point, I bring up BitKeeper:
I admit I don't know much about bitkeeper, except its license is a bit
weird...
Supports everything that CVS doesn't, including rename events...
So does BitKeeper :)
BTW - I'm not _seriously_ suggesting this change - but it would be cool,
wouldn't it?People could start their own local branches which are part of the global
namespace, easily merge them in, etc...This seems quite pointless for PostgreSQL's development.
NOT TRUE!!!
Imagine you want to develop a massive new feature for Postgres. You just
create a branch on your own machine, do all your changes, commits, etc. and
keep it current with the main branch. Then, you can merge it back into the
main tree... That way you can have a history of commits on your own branch
of the repo!
Disclaimer: Have only read docs, not actually _used_ 'arch'... :(
Chris
On Thu, 2002-02-28 at 06:44, Tom Lane wrote:
Hmm. I'd surely be the last to claim that CVS is the be-all and end-all
of software archiving systems. But is BitKeeper, or arch, or anything
else enough better to justify the pain of switching? This is intended
as an honest question, not flamebait --- I haven't looked closely at
the alternatives.
There was a discussion on Slashdot a couple of weeks ago, with (as
usual) some interesting comments out of the lot. It's at
http://slashdot.org/comments.pl?sid=27540&threshold=4&mode=flat
including comments from Subversion's Karl Fogel.
--
Alessio F. Bragadini alessio@albourne.com
APL Financial Services http://village.albourne.com
Nicosia, Cyprus phone: +357-22-755750
"It is more complicated than you think"
-- The Eighth Networking Truth from RFC 1925
Imagine you want to develop a massive new feature for Postgres. You
just
create a branch on your own machine, do all your changes, commits,
etc. and
keep it current with the main branch. Then, you can merge it back
into the
main tree... That way you can have a history of commits on your own
branch
of the repo!
The same thing can be accomplished with CVS as well -- it's just not
as pretty. There is a reason that the FreeBSD group uses $FreeBSD$
and leaves $Id$ untouched.
Basically, check out of one, drop CVS directories, check into the
second, check out of the second, and when doing work with either
repository you specify which repo with the -D flag. Coupled with
the -j (merge) flag you can accomplish most tasks.
That said, if the work was thought through and beneficial you may be
able to obtain a branch in postgresql cvs to work with.