command.c breakup

Started by Christopher Kings-Lynnealmost 24 years ago12 messages

Christopher Kings-Lynne

chriskl@familyhealth.com.au

almost 24 years ago

Hi All,

With regards to the proposed command.c refactoring...

I've done it by removing command.c and replacing it with

portal.c
alter.c
lock.c
namespace.c

Is that a good idea? Will it break too many outstanding patches?

Basically the portal fetch/destroy commands go in portal.c, all the Alter*
commands with their static helper functions go in alter.c, the single
LockTable command goes in lock.c and the CreateSchema function goes in
namespace.c. I anticipate that a few more functions will eventually be
created to go in namespace.c

I have also broken up the command.h header file into four separate
correspondingly named header files, and removed command.h itself.

The next step after this would be to move a lot of the redundant code in
alter.c into static functions.

Thoughts?

Chris

John Gray

jgray@azuli.co.uk

almost 24 years ago

In reply to: Christopher Kings-Lynne (#1)

Re: command.c breakup

On Wed, 2002-04-03 at 09:39, Christopher Kings-Lynne wrote:

Hi All,

With regards to the proposed command.c refactoring...

..about which I should apologise as I stuck my head above the parapet
and then sat on my ideas (mixing metaphors a bit).

I've done it by removing command.c and replacing it with

portal.c
alter.c
lock.c
namespace.c

Is that a good idea? Will it break too many outstanding patches?

The feedback I had was not to worry too much about that! However, my
scheme doesn't take account of some of the more recent changes -I had
envisaged a more radical division by "object manipulated". Here's my
current working draft (doesn't include material from the last couple of
weeks):

command.c
---------

PortalCleanup
PerformPortalFetch
PerformPortalClose
Portal support functions move to portal.c

AlterTableAddColumn
AlterTableAlterColumnDefault
drop_default
AlterTableAlterColumnFlags

These move to table.c. They share common code for permissions
and recursion. Therefore, propose to create a short helper
routine (AlterTableAlterColumnSetup) which checks permissions,
existence of relation (and acquirtes lock on rel?). Also
provide macros for recursion, to be used in form:

RECURSE_OVER_CHILDREN(relid);
AlterTableDoSomething(args);
RECURSE_OVER_CHILDREN_END;

find_attribute_walker
find_attribute_in_node
RemoveColumnReferences
AlterTableDropColumn

These are part of the old DROP_COLUMN_HACK. Should they go in
the transfer? (There seems to be agreement that DROP COLUMN
will not be implemented as it is here).

AlterTableAddConstraint
AlterTableDropConstraint

Move to table.c These also use permissions and recursion code.

AlterTableOwner
AlterTableCreateToastTable
needs_toast_table
All move to table.c. (Seems a bit more drastic than necessary
to split AlterTableCreateToastTable and move
needs_toast_table to access/heap/tuptoaster.c).

LockTableCommand
Move to lock.c

creatinh.c
----------

DefineRelation
RemoveRelation
TruncateRelation
MergeAttributes
change_varattnos_walker
change_varattnos_of_a_node
StoreCatalogInheritance
findAttrByName
setRelhassubclassInRelation

All move to table.c

define.c
--------

case_translate_language_name

Remove this one and refer to that in proclang.c

compute_return_type
compute_full_attributes
interpret_AS_clause
CreateFunction

Move to function.c

DefineOperator

Move to operator.c

DefineAggregate

Move to aggregate.c

DefineType

Move to type.c

defGetString
defGetNumeric
defGetTypeLength

Parameter fetching support, generic to all the processing for
define statements. Inclined to move to type.c as used most by type
creation.

remove.c
--------

RemoveOperator

To operator.c

SingleOpOperatorRemove
AttributeAndRelationRemove

To operator.c (or delete altogether -NOTYET since 94!)

RemoveType

To type.c

RemoveFunction

To function.c

RemoveAggregate

To aggregate.c

rename.c
--------

renameatt
renamerel
ri_trigger_type
update_ri_trigger_args

To table.c

Thus, the change in the set of files:

Removed:

command.c
creatinh.c
define.c
remove.c
rename.c

Added:
aggregate.c
function.c
operator.c
table.c
type.c

Sorry for going slow on this - but it seems that the organisation
has dropped out of my life in the last few weeks :) (and I've been away
over Easter).

Regards

John

Tom Lane

tgl@sss.pgh.pa.us

almost 24 years ago

In reply to: John Gray (#2)

Re: command.c breakup

John Gray <jgray@azuli.co.uk> writes:

Here's my current working draft (doesn't include material from the
last couple of weeks):

Please note that there's been pretty substantial revisions in command.c
and creatinh.c over the past couple of weeks for schema support. While
I think that those two files are largely done with, define.c and
remove.c are about to get the same treatment as the schema project moves
on to schema-tizing functions and operators. So we'll need to coordinate
just when and how to make these structural revisions; and you'll
definitely need to be working against CVS tip. What are your plans,
time-wise? Does it make sense for the two of you to work together?

These are part of the old DROP_COLUMN_HACK. Should they go in
the transfer? (There seems to be agreement that DROP COLUMN
will not be implemented as it is here).

I think Hiroshi finally removed all the DROP_COLUMN_HACK code yesterday.

Parameter fetching support, generic to all the processing for
define statements. Inclined to move to type.c as used most by type
creation.

What about leaving define.c in existence, but have it hold only common
support routines for object-definition commands? The param fetchers
would certainly fit in this category, and maybe some of the other
support routines you've described would fit here too.

To operator.c (or delete altogether -NOTYET since 94!)

NOTYET probably means NEVER; whenever that functionality is implemented,
it'll be based on some sort of generic dependency code, not
special-purpose checks. Feel free to remove this stuff too.

Thus, the change in the set of files:

Removed:

command.c
creatinh.c
define.c
remove.c
rename.c

Added:
aggregate.c
function.c
operator.c
table.c
type.c

Minor gripe here: I would suggest taking a cue from indexcmds.c and
choosing file names along the lines of functioncmds.c, tablecmds.c,
etc. The above names strike me as too generic and likely to cause
confusion with similarly-named files in other directories.

Sorry for going slow on this - but it seems that the organisation
has dropped out of my life in the last few weeks :) (and I've been away
over Easter).

Not a problem. But we'll need a concentrated burst of work whenever
you are ready to prepare the final version of the patch; otherwise the
synchronization issues will cause problems/delays for other people.

regards, tom lane

John Gray

jgray@azuli.co.uk

almost 24 years ago

In reply to: Tom Lane (#3)

Re: command.c breakup

On Wed, 2002-04-03 at 16:52, Tom Lane wrote:

John Gray <jgray@azuli.co.uk> writes:

Here's my current working draft (doesn't include material from the
last couple of weeks):

Please note that there's been pretty substantial revisions in command.c
and creatinh.c over the past couple of weeks for schema support. While
I think that those two files are largely done with, define.c and
remove.c are about to get the same treatment as the schema project moves
on to schema-tizing functions and operators. So we'll need to coordinate
just when and how to make these structural revisions; and you'll
definitely need to be working against CVS tip. What are your plans,
time-wise? Does it make sense for the two of you to work together?

I have compiled a new version against current CVS, now also including
references to dependencies (See below). I accept that we'll need to work
round the schema project -in the week since the last message I notice
that namespace support has arrived for function, aggregate and operator
creation. Is there more to come in these files?

I'm unsure whether it is sensible to split the commands/defrem.h file to
match the actual .c files (given that there are at present only two
externally referenced functions from each entity it seems reasonable to
keep them together -as they are all referred to from tcop/utility.c
anyway.

As far as joint working goes, if Chris K-L would like to grab all or
part of it he is very welcome :) My timescale is that I have time at
present to work on it, so maybe next week for incorporation (but do
people need more notice than that?)

Obviously, I haven't given more details of the common code elimination.
That is a slightly different kind of task -I'll post some specifics on
that in the next couple of days.

Parameter fetching support, generic to all the processing for
define statements. Inclined to move to type.c as used most by type
creation.

What about leaving define.c in existence, but have it hold only common
support routines for object-definition commands? The param fetchers
would certainly fit in this category, and maybe some of the other
support routines you've described would fit here too.

Yes, this seems sensible -but as far as the other support code goes, it
might make sense to have a file called (say) cmdsupport.c where the
parameter fetchers, the checking and recursion code etc. all goes?

To operator.c (or delete altogether -NOTYET since 94!)

NOTYET probably means NEVER; whenever that functionality is implemented,
it'll be based on some sort of generic dependency code, not
special-purpose checks. Feel free to remove this stuff too.

Thus, the change in the set of files:

Minor gripe here: I would suggest taking a cue from indexcmds.c and
choosing file names along the lines of functioncmds.c, tablecmds.c,
etc. The above names strike me as too generic and likely to cause
confusion with similarly-named files in other directories.

Yes, this makes sense and I've done that too.

Sorry for going slow on this - but it seems that the organisation
has dropped out of my life in the last few weeks :) (and I've been away
over Easter).

Not a problem. But we'll need a concentrated burst of work whenever
you are ready to prepare the final version of the patch; otherwise the
synchronization issues will cause problems/delays for other people.

That shouldn't be too much of a problem in the next couple of weeks - if
we can decide on a specific day I'll book it into my diary (Any day but
Wednesday next week would be fine for me).

Regards

John

src/backend/commands/ directory reorganisation version 2
(including dependencies), from CVS as of 12 noon, 2002-04-11)

Dependencies were determined from LXR cross-reference database. This
will show all *usage* -it won't catch cases where a header file is included
redundantly. Recursive grep seems to provide the same answers though.

command.c
---------

PortalCleanup
PerformPortalFetch
PerformPortalClose
Portal support functions move to portalcmds.c

prototype commands/command.h -> commands/portal.h
refer executor/spi.c tcop/pquery.c tcop/utility.c

AlterTableAddColumn
AlterTableAlterColumnDropNotNull
AlterTableAlterColumnSetNotNull
AlterTableAlterColumnDefault
drop_default
AlterTableAlterColumnFlags
AlterTableDropColumn
AlterTableAddConstraint
AlterTableDropConstraint
AlterTableOwner
AlterTableCreateToastTable
needs_toast_table

These move to tablecmds.c. They share common code for permissions
and recursion. Therefore, propose to create a short helper
routine (AlterTableAlterColumnSetup) which checks permissions,
existence of relation (and acquirtes lock on rel?). Also
provide macros for recursion, to be used in form:

RECURSE_OVER_CHILDREN(relid);
AlterTableDoSomething(args);
RECURSE_OVER_CHILDREN_END;

prototype commands/command.h -> commands/tablecmds.h
refer tcop/utility.c commands/cluster.c executor/execMain.c

LockTableCommand
Move to lockcmds.c

prototype commands/command.h -> commands/lockcmds.h
refer tcop/utility.c

CreateSchemaCommand
Move to schemacmds.c

prototype commands/command.h -> commands/schemacmds.h
refer tcop/utility.c

creatinh.c
----------

DefineRelation
RemoveRelation
TruncateRelation
MergeDomainAttributes
MergeAttributes
change_varattnos_walker
change_varattnos_of_a_node
StoreCatalogInheritance
findAttrByName
setRelhassubclassInRelation

All move to tablecmds.c

prototye commands/creatinh.h -> commands/tablecmds.h
refer commands/sequence.c commands/view.c tcop/utility.c

define.c
--------

case_translate_language_name

Remove this one and refer to that in proclang.c. If this file
becomes a file for support functions, then the reverse should apply.

compute_return_type
compute_full_attributes
interpret_AS_clause
CreateFunction

Move to functioncmds.c