On Mon, Jun 26, 2006 at 12:31:24PM -0400, Greg Stark wrote:
> Currently analyze.c and tablecmds.c both have some very similar code to handle
> copying columns from "parent" tables. As long as they were just copying
> columns and in the case of tablecmds.c copying check constraints that wasn't
> really that bad an idea, the code is pretty simple.
> However with partitioned tables it becomes important to copy more table
> attributes like foreign key constraints and hopefully one day indexes. And it
> would be awfully convenient to have CREATE TABLE LIKE have options to copy the
> same things that inherited tables get. And have exactly the same semantics.

People will turn around and immediately as for create table like without us
making the assumptions it wanted all of the extras that come with inheritance.
COPY is a copy of the table, not additional functionality.  The added 
of adding only what is necessary makes more sense than going in to guess
what was added and removing it later if it is not needed.

This does not preclude adding a copy table like (with extras) though if you 

> So I'm suggesting refactoring the code from analyze.c and tablecmds.c into
> functions to copy the columns, constraints, indexes etc.
> For example I see a functions like:
> List *CopyTableColumns(relation source, List *target_schema)
> List *CopyTableCheckConstraints(relation source, List *target_schema)
> ...
> To do this though might require some changes in the algorithm used for
> inherited tables. Currently it builds up the list of merged columns
> incrementally. I'm thinking it would be more natural to accumulate all the
> columns from parents and then remove duplicates in a single pass. I think it
> should be possible to maintain precisely the same semantics doing this though.

Be careful because the code may be taking into account multiple inheritance 
I'm not sure about this. It should be looked at carefully.

> I may be able to make AddInherits a consumer for these functions as well,
> though it would be a bit awkward since it would have to construct a fake list
> of ColumnDefs to act as the target schema. It would have the side effect of
> making the constraint comparison use change_varattnos_of_a_node and then
> compare the binary representations rather than decompiling the constraints to
> do the comparison. I'm not sure if that's the same semantics.
Again, be sure it is the same semantics before going that way.
> To a certain degree I feel like this is just make-work. The existing code
> works fine and I can just happily keep additing functionality to both
> analyze.c and tablecmds.c. And it's possible we won't always want to have the
> two match.
> Has anyone looked at applying the ADD INHERITS patch yet? Would it be more or
> less likely to be accepted if it were a bigger patch that refactored all this
> stuff like I'm talking about?
> -- 
> greg

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?


Reply via email to