> > I will really try to see if we have other issues. Really cannot have
> Robert
> > reporting all the bugs and getting annoyed, but there are lotsa
> variations
> > possible with inheritance..
>
> BTW thank you for doing the work on this. Probably the reason no one
> bothers too much with inheri
> > I have also tried to change the error message as per Alvaro's
> suggestions.
> > I will really try to see if we have other issues. Really cannot have
> Robert
> > reporting all the bugs and getting annoyed, but there are lotsa
> variations
> > possible with inheritance..
>
> So did you find any
Excerpts from Nikhil Sontakke's message of vie dic 23 01:02:10 -0300 2011:
> I have also tried to change the error message as per Alvaro's suggestions.
> I will really try to see if we have other issues. Really cannot have Robert
> reporting all the bugs and getting annoyed, but there are lotsa v
Excerpts from Nikhil Sontakke's message of vie dic 23 01:02:10 -0300 2011:
> FWIW, here's a quick fix for the issue that Robert pointed out.
Thanks, applied.
> Again it's
> a variation of the first issue that he reported. We have two functions
> which try to merge existing constraints:
>
> Mer
> > I don't think this is a given ... In fact, IMO if we're only two or
> > three fixes away from having it all nice and consistent, I think
> > reverting is not necessary.
>
> Sure. It's the "if" part of that sentence that I'm not too sure about.
>
>
Any specific area of the code that you think
On Thu, Dec 22, 2011 at 10:54 PM, Alvaro Herrera
wrote:
> I agree with Robert that this usage of ALTER TABLE ONLY is slightly
> different from other usages of the same command, but I disagree that
> this means that we need another command to do what we want to do here.
> IOW, I prefer to keep the
> I don't think this is a given ... In fact, IMO if we're only two or
> three fixes away from having it all nice and consistent, I think
> reverting is not necessary.
>
>
FWIW, here's a quick fix for the issue that Robert pointed out. Again it's
a variation of the first issue that he reported. We
Excerpts from Nikhil Sontakke's message of vie dic 23 00:25:26 -0300 2011:
> Hi,
>
> > There is at least one other
> > problem. Consider:
> >
> > rhaas=# create table a (ff1 int, constraint chk check (ff1 > 0));
> > CREATE TABLE
> > rhaas=# create table b (ff1 int, constraint chk check (ff1 > 0)
And yeah, certainly there's a bug as you point out.
postgres=# create table a1 (ff1 int, constraint chk check (ff1 > 0));
postgres=# create table b1 (ff1 int);
postgres=# alter table only b1 add constraint chk check (ff1 > 0);
postgres=# alter table b1 inherit a1;
The last command should have ref
Hi,
> There is at least one other
> problem. Consider:
>
> rhaas=# create table a (ff1 int, constraint chk check (ff1 > 0));
> CREATE TABLE
> rhaas=# create table b (ff1 int, constraint chk check (ff1 > 0));
> CREATE TABLE
> rhaas=# alter table b inherit a;
> ALTER TABLE
>
> This needs to fail i
On Thu, Dec 22, 2011 at 2:43 PM, Alvaro Herrera
wrote:
> Excerpts from Nikhil Sontakke's message of mar dic 20 12:03:33 -0300 2011:
>
>> > Apologies, I did not check this particular scenario.
>> >
>> > I guess, here, we should not allow merging of the inherited constraint
>> > into an "only" const
Excerpts from Nikhil Sontakke's message of mar dic 20 12:03:33 -0300 2011:
> > Apologies, I did not check this particular scenario.
> >
> > I guess, here, we should not allow merging of the inherited constraint
> > into an "only" constraint. Because that breaks the semantics for "only"
> > constr
> rhaas=# create table A(ff1 int);
>> CREATE TABLE
>> rhaas=# create table B () inherits (A);
>> CREATE TABLE
>> rhaas=# create table C () inherits (B);
>> CREATE TABLE
>> rhaas=# alter table only b add constraint chk check (ff1 > 0);
>> ALTER TABLE
>> rhaas=# alter table a add constraint chk check
> > Agreed. I just tried out the scenarios laid out by you both with and
> without
> > the committed patch and AFAICS, normal inheritance semantics have been
> > preserved properly even after the commit.
>
> No, they haven't. I didn't expect this to break anything when you
> have two constraints w
On Tue, Dec 20, 2011 at 1:14 AM, Nikhil Sontakke wrote:
> Agreed. I just tried out the scenarios laid out by you both with and without
> the committed patch and AFAICS, normal inheritance semantics have been
> preserved properly even after the commit.
No, they haven't. I didn't expect this to br
Hi Robert,
First of all, let me state that this "ONLY" feature has not messed around
with existing inheritance semantics. It allows attaching a constraint to
any table (which can be part of any hierarchy) without the possibility of
it ever playing any part in future or existing inheritance hierarc
> But did you see Robert Haas' response upthread? It looks like there's
> still some work to do -- at least some research to determine that we're
> correctly handling all those cases. As the committer I'm responsible
> for it, but I don't have resources to get into it any time soon.
>
>
Yeah, am
Excerpts from Nikhil Sontakke's message of lun dic 19 22:32:57 -0300 2011:
> > > PFA, revised version containing the above changes based on Alvaro's v4
> > > patch.
> >
> > Committed with these fixes, and with my fix for the pg_dump issue noted
> > by Alex, too. Thanks.
> >
> Thanks a lot Alvaro!
> > PFA, revised version containing the above changes based on Alvaro's v4
> > patch.
>
> Committed with these fixes, and with my fix for the pg_dump issue noted
> by Alex, too. Thanks.
>
>
Thanks a lot Alvaro!
Regards,
Nikhils
> --
> Álvaro Herrera
> The PostgreSQL Company - Command Prompt, I
Excerpts from Nikhil Sontakke's message of lun dic 19 14:07:02 -0300 2011:
> PFA, revised version containing the above changes based on Alvaro's v4
> patch.
Committed with these fixes, and with my fix for the pg_dump issue noted
by Alex, too. Thanks.
--
Álvaro Herrera
The PostgreSQL Company
On Mon, Dec 19, 2011 at 12:07 PM, Nikhil Sontakke wrote:
>> It seems hard to believe that ATExecDropConstraint() doesn't need any
>> adjustment.
>
> Yeah, I think we could return early on for "only" type of constraints.
It's not just that. Suppose that C inherits from B which inherits
from A. W
Excerpts from Alex Hunsaker's message of vie dic 16 18:07:05 -0300 2011:
>
> On Fri, Dec 16, 2011 at 14:01, Alvaro Herrera
> wrote:
> >
> > Excerpts from Alex Hunsaker's message of vie dic 16 17:50:12 -0300 2011:
> >>
> >> On Fri, Dec 16, 2011 at 12:06, Alvaro Herrera
> >> wrote:
> >>
> >> > Ye
> It seems hard to believe that ATExecDropConstraint() doesn't need any
> adjustment.
>
>
Yeah, I think we could return early on for "only" type of constraints.
Also, shouldn't the systable scan break out of the while loop after a
matching constraint name has been found? As of now, it's doing a fu
On Fri, Dec 16, 2011 at 2:06 PM, Alvaro Herrera
wrote:
> Yeah. Nikhil, Alex, this is the merged patch. Have a look that it
> still works for you (particularly the pg_dump bits) and I'll commit it.
> I adjusted the regression test a bit too.
It seems hard to believe that ATExecDropConstraint() d
Hi Alvaro,
The patch looks ok to me. I see that we now sort the constraints by
conisonly value too:
@@ -1781,12 +1781,20 @@ describeOneTableDetails(const char *schemaname,
/* print table (and column) check constraints */
if (tableinfo.checks)
{
+char *is_onl
On Fri, Dec 16, 2011 at 14:01, Alvaro Herrera
wrote:
>
> Excerpts from Alex Hunsaker's message of vie dic 16 17:50:12 -0300 2011:
>>
>> On Fri, Dec 16, 2011 at 12:06, Alvaro Herrera
>> wrote:
>>
>> > Yeah. Nikhil, Alex, this is the merged patch. Have a look that it
>> > still works for you (par
Excerpts from Alex Hunsaker's message of vie dic 16 17:50:12 -0300 2011:
>
> On Fri, Dec 16, 2011 at 12:06, Alvaro Herrera
> wrote:
>
> > Yeah. Nikhil, Alex, this is the merged patch. Have a look that it
> > still works for you (particularly the pg_dump bits) and I'll commit it.
> > I adjuste
On Fri, Dec 16, 2011 at 12:06, Alvaro Herrera
wrote:
> Yeah. Nikhil, Alex, this is the merged patch. Have a look that it
> still works for you (particularly the pg_dump bits) and I'll commit it.
> I adjusted the regression test a bit too.
Other than the version checks seem to be off by one loo
Excerpts from Greg Smith's message of vie dic 16 15:02:20 -0300 2011:
> On 12/04/2011 02:22 AM, Nikhil Sontakke wrote:
> >
> > Is it okay to modify an existing constraint to mark it as "only", even
> > if it was originally inheritable? This is not clear to me. Maybe the
> > safest cou
On 12/04/2011 02:22 AM, Nikhil Sontakke wrote:
Is it okay to modify an existing constraint to mark it as "only", even
if it was originally inheritable? This is not clear to me. Maybe the
safest course of action is to raise an error. Or maybe I'm misreading
what it does (becaus
> I had a look at this patch today. The pg_dump bits conflict with
> another patch I committed a few days ago, so I'm about to merge them.
> I have one question which is about this hunk:
>
>
Thanks for taking a look Alvaro.
> @@ -2312,6 +2317,11 @@ MergeWithExistingConstraint(Relation rel, char
Excerpts from Alex Hunsaker's message of dom oct 09 03:40:36 -0300 2011:
> On Fri, Oct 7, 2011 at 21:30, Nikhil Sontakke wrote:
> > Hi Alex,
> >
> > I guess we both are in agreement with each other :)
> >
> > After sleeping over it, I think that check is indeed dead code with this new
> > non-inh
On Fri, Oct 7, 2011 at 21:30, Nikhil Sontakke wrote:
> Hi Alex,
>
> I guess we both are in agreement with each other :)
>
> After sleeping over it, I think that check is indeed dead code with this new
> non-inheritable check constraints functionality in place. So unless you have
> some other comme
Hi Alex,
I guess we both are in agreement with each other :)
After sleeping over it, I think that check is indeed dead code with this new
non-inheritable check constraints functionality in place. So unless you have
some other comments, we can mark this as 'Ready for Commiter'.
Again, thanks for
On Fri, Oct 7, 2011 at 00:28, Nikhil Sontakke wrote:
> Hi Alex,
>> So with it all spelled out now I see the "constraint must be added to
>> child tables too" check is dead code.
>>
>
> Thanks the above step-wise explanation helps.
>
> But AFAICS, the default inhOpt value can be governed by the SQ
Hi Alex,
> Hmmm, your patch checks for a constraint being "only" via:
> >
> > !recurse && !recursing
> >
> > I hope that is good enough to conclusively conclude that the constraint
> is
> > 'only'. This check was not too readable in the existing code for me
> anyways
> > ;). If we ch
On Thu, Oct 6, 2011 at 02:42, Nikhil Sontakke wrote:
> Hi Alex,
>
>> I didn't care for the changes to gram.y so I reworked it a bit so we
>> now pass is_only to AddRelationNewConstraint() (like we do with
>> is_local). Seemed simpler but maybe I missed something. Comments?
>>
>
> Hmmm, your patch
Hi Alex,
I didn't care for the changes to gram.y so I reworked it a bit so we
> now pass is_only to AddRelationNewConstraint() (like we do with
> is_local). Seemed simpler but maybe I missed something. Comments?
>
>
Hmmm, your patch checks for a constraint being "only" via:
!recurse
Hi! *Waves*
First off, it all seems to work as described:
- regressions pass
- domains work
- tried various inherit options (merging constraints, alter table no
inherit etc)
- pg_dump/restore
I didn't care for the changes to gram.y so I reworked it a bit so we
now pass is_only to AddRelationNewCo
39 matches
Mail list logo
