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:
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
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 anything?
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 inheritance is
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 is/has
On Thu, Dec 22, 2011 at 10:54 PM, Alvaro Herrera
alvhe...@commandprompt.com 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.
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
constraints. If
On Thu, Dec 22, 2011 at 2:43 PM, Alvaro Herrera
alvhe...@commandprompt.com 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
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 if chk is an
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
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));
CREATE
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 have
On Tue, Dec 20, 2011 at 1:14 AM, Nikhil Sontakke nikkh...@gmail.com 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
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 with
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 (ff1 0);
On Fri, Dec 16, 2011 at 2:06 PM, Alvaro Herrera
alvhe...@commandprompt.com 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
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 full
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
alvhe...@commandprompt.com 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
On Mon, Dec 19, 2011 at 12:07 PM, Nikhil Sontakke nikkh...@gmail.com 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
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
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 alvhe...@commandprompt.com
The PostgreSQL Company -
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!
You're
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
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
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
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 (because
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 course of
On Fri, Dec 16, 2011 at 12:06, Alvaro Herrera
alvhe...@commandprompt.com 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
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
alvhe...@commandprompt.com 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
On Fri, Dec 16, 2011 at 14:01, Alvaro Herrera
alvhe...@commandprompt.com 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
alvhe...@commandprompt.com wrote:
Yeah. Nikhil, Alex, this is the merged patch. Have a
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 nikkh...@gmail.com 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
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
On Fri, Oct 7, 2011 at 21:30, Nikhil Sontakke nikkh...@gmail.com 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
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 check at the grammar
On Fri, Oct 7, 2011 at 00:28, Nikhil Sontakke nikkh...@gmail.com 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
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
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
On Thu, Oct 6, 2011 at 02:42, Nikhil Sontakke nikkh...@gmail.com 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
38 matches
Mail list logo
