ADD/DROPS INHERIT (actually INHERIT / NO INHERIT)

I cleaned up the code and added some more documentation.

I think I've addressed all the concerns raised so far. Please tell me if I've
missed anything.

There were a few tangentially related issues that have come up that I think
are TODOs. I'm likely to tackle one or two of these next so I'm interested in
hearing feedback on them as well.

. Constraints currently do not know anything about inheritance. Tom suggested
adding a coninhcount and conislocal like attributes have to track their
inheritance status.

. Foreign key constraints currently do not get copied to new children (and
therefore my code doesn't verify them). I don't think it would be hard to
add them and treat them like CHECK constraints.

. No constraints at all are copied to tables defined with LIKE. That makes it
hard to use LIKE to define new partitions. The standard defines LIKE and
specifically says it does not copy constraints. But the standard already has
an option called INCLUDING DEFAULTS; we could always define a non-standard
extension LIKE table INCLUDING CONSTRAINTS that gives the user the option to
request a copy including constraints.

. Personally, I think the whole attislocal thing is bunk. The decision about
whether to drop a column from children tables or not is something that
should be up to the user and trying to DWIM based on whether there was ever
a local definition or the column was acquired purely through inheritance is
hardly ever going to match up with user expectations.

. And of course there's the whole unique and primary key constraint issue. I
think to get any traction at all on this you have a prerequisite of a real
partitioned table implementation where the system knows what the partition
key is so it can recognize when it's a leading part of an index key.

Patch applied. Thanks.

I ran pgindent on the tablecmds.c block of code, and cleaned up some
boolean assignments. There are a few XXX comments still in the code so
someone should look at those questions and either modify the code or
remove the comments.

---------------------------------------------------------------------------

Greg Stark wrote:
>
> I cleaned up the code and added some more documentation.
>
> I think I've addressed all the concerns raised so far. Please tell me if I've
> missed anything.
>
> There were a few tangentially related issues that have come up that I think
> are TODOs. I'm likely to tackle one or two of these next so I'm interested in
> hearing feedback on them as well.
>
> . Constraints currently do not know anything about inheritance. Tom suggested
> adding a coninhcount and conislocal like attributes have to track their
> inheritance status.
>
> . Foreign key constraints currently do not get copied to new children (and
> therefore my code doesn't verify them). I don't think it would be hard to
> add them and treat them like CHECK constraints.
>
> . No constraints at all are copied to tables defined with LIKE. That makes it
> hard to use LIKE to define new partitions. The standard defines LIKE and
> specifically says it does not copy constraints. But the standard already has
> an option called INCLUDING DEFAULTS; we could always define a non-standard
> extension LIKE table INCLUDING CONSTRAINTS that gives the user the option to
> request a copy including constraints.
>
> . Personally, I think the whole attislocal thing is bunk. The decision about
> whether to drop a column from children tables or not is something that
> should be up to the user and trying to DWIM based on whether there was ever
> a local definition or the column was acquired purely through inheritance is
> hardly ever going to match up with user expectations.
>
> . And of course there's the whole unique and primary key constraint issue. I
> think to get any traction at all on this you have a prerequisite of a real
> partitioned table implementation where the system knows what the partition
> key is so it can recognize when it's a leading part of an index key.
>
>

[ Attachment, skipping... ]

>
>
> --
> greg

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> I ran pgindent on the tablecmds.c block of code, and cleaned up some
> boolean assignments. There are a few XXX comments still in the code so
> someone should look at those questions and either modify the code or
> remove the comments.

So this patch was by no stretch of the imagination ready to apply,
but you did it anyway.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > I ran pgindent on the tablecmds.c block of code, and cleaned up some
> > boolean assignments. There are a few XXX comments still in the code so
> > someone should look at those questions and either modify the code or
> > remove the comments.
>
> So this patch was by no stretch of the imagination ready to apply,
> but you did it anyway.

Right. What is your next question?

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> So this patch was by no stretch of the imagination ready to apply,
>> but you did it anyway.

> Right. What is your next question?

Perhaps "why is the buildfarm failing" would be appropriate.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> So this patch was by no stretch of the imagination ready to apply,
> >> but you did it anyway.
>
> > Right. What is your next question?
>
> Perhaps "why is the buildfarm failing" would be appropriate.

Yes, that is appropriate, though it seems Neil's cleanup of the patch
has fixed it now. I see only a single stats failure and an initdb
failure in the buildfarm, neither of which I assume are related to the
patch.

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Perhaps "why is the buildfarm failing" would be appropriate.

> Yes, that is appropriate, though it seems Neil's cleanup of the patch
> has fixed it now. I see only a single stats failure and an initdb
> failure in the buildfarm, neither of which I assume are related to the
> patch.

Nah, it was a false alarm: I was looking at the first post-patch report,
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mongoose&dt=2006-07-02%2003:30:01
but apparently mongoose had managed to pick up a partially-updated
snapshot. The later reports (including mongoose's own next try an
hour later) were all OK.

Sorry for the noise.

regards, tom lane

On Sun, 2 Jul 2006, Tom Lane wrote:

> Nah, it was a false alarm: I was looking at the first post-patch report,
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mongoose&dt=2006-07-02%2003:30:01
> but apparently mongoose had managed to pick up a partially-updated
> snapshot. The later reports (including mongoose's own next try an
> hour later) were all OK.

As the keeper of mongoose, is there anything I should do to prevent it
from picking up a partially-updated snapshot? Or is this just a race
condition that's bound to happen now and then?

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> Perhaps "why is the buildfarm failing" would be appropriate.
>
> > Yes, that is appropriate, though it seems Neil's cleanup of the patch
> > has fixed it now. I see only a single stats failure and an initdb
> > failure in the buildfarm, neither of which I assume are related to the
> > patch.
>
> Nah, it was a false alarm: I was looking at the first post-patch report,
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mongoose&dt=2006-07-02%2003:30:01
> but apparently mongoose had managed to pick up a partially-updated
> snapshot. The later reports (including mongoose's own next try an
> hour later) were all OK.
>
> Sorry for the noise.

Thanks for keeping an eye on that buildfarm. I often forget to look
myself.

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Jeremy Drake <pgsql-patches(at)jdrake(dot)com> writes:
> On Sun, 2 Jul 2006, Tom Lane wrote:
>> Nah, it was a false alarm: I was looking at the first post-patch report,
>> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mongoose&dt=2006-07-02%2003:30:01
>> but apparently mongoose had managed to pick up a partially-updated
>> snapshot. The later reports (including mongoose's own next try an
>> hour later) were all OK.

> As the keeper of mongoose, is there anything I should do to prevent it
> from picking up a partially-updated snapshot? Or is this just a race
> condition that's bound to happen now and then?

Well, it's certainly not *your* problem to fix. I suspect that this
risk is inherent in CVS --- although there might also be something
involved about our primary-vs-mirror CVS setup. Does anyone know
exactly how the mirroring is done and whether it makes any attempt to
ensure a consistent copy?

regards, tom lane

Tom Lane said:
> Jeremy Drake <pgsql-patches(at)jdrake(dot)com> writes:
>> On Sun, 2 Jul 2006, Tom Lane wrote:
>>> Nah, it was a false alarm: I was looking at the first post-patch
>>> report,
>>>
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mongoose&dt=2006-07-02%2003:30:01>>> but apparently mongoose had managed to pick up a partially-updated
>>> snapshot. The later reports (including mongoose's own next try an
>>> hour later) were all OK.
>
>> As the keeper of mongoose, is there anything I should do to prevent it
>> from picking up a partially-updated snapshot? Or is this just a race
>> condition that's bound to happen now and then?
>
> Well, it's certainly not *your* problem to fix. I suspect that this
> risk is inherent in CVS --- although there might also be something
> involved about our primary-vs-mirror CVS setup. Does anyone know
> exactly how the mirroring is done and whether it makes any attempt to
> ensure a consistent copy?
>

Since CVS updates are not atomic, it's hard to see how mirroring could be,
unless you did something like disallow updates, mirror, allow updates. I
suspect such a cure would be worse than the disease. This is such a rare
event that I don't think it's worth the trouble. Buildfarm members are doing
200 builds a day or more, and I can't recall having seen this before.

cheers

andrew

"Andrew Dunstan" <andrew(at)dunslane(dot)net> writes:
> Since CVS updates are not atomic, it's hard to see how mirroring could be,
> unless you did something like disallow updates, mirror, allow updates. I
> suspect such a cure would be worse than the disease. This is such a rare
> event that I don't think it's worth the trouble. Buildfarm members are doing
> 200 builds a day or more, and I can't recall having seen this before.

Yeah, I don't remember having seen it before either, but on the other
hand I haven't been paying super close attention.

One easy low-tech fix would be for Marc to publish the exact times at
which the mirror syncs run (I think it might be something like 20 past
the hour but I'm not sure), and then we could tell buildfarm owners not
to schedule their CVS pulls to start in that particular five-minute
window, and committers could try to avoid committing many-file patches
right then either.

regards, tom lane

[redirecting to -hackers]

context: a buildfarm member apparently failed through getting a partial
update from CVS, possibly because the anonymous mirror was also
partially updated.

Tom Lane wrote:

>"Andrew Dunstan" <andrew(at)dunslane(dot)net> writes:
>
>
>>Since CVS updates are not atomic, it's hard to see how mirroring could be,
>>unless you did something like disallow updates, mirror, allow updates. I
>>suspect such a cure would be worse than the disease. This is such a rare
>>event that I don't think it's worth the trouble. Buildfarm members are doing
>>200 builds a day or more, and I can't recall having seen this before.
>>
>>
>
>Yeah, I don't remember having seen it before either, but on the other
>hand I haven't been paying super close attention.
>
>One easy low-tech fix would be for Marc to publish the exact times at
>which the mirror syncs run (I think it might be something like 20 past
>the hour but I'm not sure), and then we could tell buildfarm owners not
>to schedule their CVS pulls to start in that particular five-minute
>window, and committers could try to avoid committing many-file patches
>right then either.
>
>
>
>

Yuck. I think if it gets that far we would have discovered a compelling
reason to abandon CVS, as many bystanders have urged us to do. But I
think we can live with an occasional hiccup.

cheers

andrew

19 minutes past the hour, every hour ...

On Mon, 3 Jul 2006, Andrew Dunstan wrote:

>
> [redirecting to -hackers]
>
> context: a buildfarm member apparently failed through getting a partial
> update from CVS, possibly because the anonymous mirror was also partially
> updated.
>
> Tom Lane wrote:
>
>> "Andrew Dunstan" <andrew(at)dunslane(dot)net> writes:
>>
>>> Since CVS updates are not atomic, it's hard to see how mirroring could be,
>>> unless you did something like disallow updates, mirror, allow updates. I
>>> suspect such a cure would be worse than the disease. This is such a rare
>>> event that I don't think it's worth the trouble. Buildfarm members are
>>> doing
>>> 200 builds a day or more, and I can't recall having seen this before.
>>>
>>
>> Yeah, I don't remember having seen it before either, but on the other
>> hand I haven't been paying super close attention.
>>
>> One easy low-tech fix would be for Marc to publish the exact times at
>> which the mirror syncs run (I think it might be something like 20 past
>> the hour but I'm not sure), and then we could tell buildfarm owners not
>> to schedule their CVS pulls to start in that particular five-minute
>> window, and committers could try to avoid committing many-file patches
>> right then either.
>>
>>
>
> Yuck. I think if it gets that far we would have discovered a compelling
> reason to abandon CVS, as many bystanders have urged us to do. But I think we
> can live with an occasional hiccup.
>
>
> cheers
>
> andrew
>
>

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email . scrappy(at)hub(dot)org MSN . scrappy(at)hub(dot)org
Yahoo . yscrappy Skype: hub.org ICQ . 7615664