Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-03 Thread Yeb Havinga
Robert Haas wrote: On Mon, Aug 2, 2010 at 2:56 PM, Yeb Havinga yebhavi...@gmail.com wrote: Hence the ATOneLevelRecursion routing is usable in its current form for all callers during the prep stage, and not only ATPrepAddColumn. Well, only if they happen to want the visit each table only once

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-03 Thread Yeb Havinga
Yeb Havinga wrote: The underlying cause is the failure of the code to recognize that if relation C inherits from both A and B, where A and B both have column x, that A.x 'is the same as' B.x, where the 'is the same as' relation is the same that holds for (A.x, C.x) and (B.x, C.x), which the

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-02 Thread Yeb Havinga
Robert Haas wrote: I agree that's the crux of the problem, but I can't see solving it with a global variable. I realize you were just testing... Yes it was just a test. However, somewhere information must be kept or altered so it can be detected that a relation has already been visited,

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-02 Thread Robert Haas
On Mon, Aug 2, 2010 at 9:20 AM, Yeb Havinga yebhavi...@gmail.com wrote: Robert Haas wrote: I agree that's the crux of the problem, but I can't see solving it with a global variable.  I realize you were just testing... Yes it was just a test. However, somewhere information must be kept or

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-02 Thread Yeb Havinga
Robert Haas wrote: On Mon, Aug 2, 2010 at 9:20 AM, Yeb Havinga yebhavi...@gmail.com wrote: The attached patch uses the globally defined list. I can't speak for any other committer, but personally I'm prepared to reject out of hand any solution involving a global variable. This code is none to

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-02 Thread Robert Haas
On Mon, Aug 2, 2010 at 10:47 AM, Yeb Havinga yebhavi...@gmail.com wrote: The attached patch uses the globally defined list. I can't speak for any other committer, but personally I'm prepared to reject out of hand any solution involving a global variable.  This code is none to easy to follow

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-02 Thread Yeb Havinga
Robert Haas wrote: I don't think that this is much cleaner than the global variable solution; you haven't really localized that need to know about the new flag in any meaningful way, the hacks in ATOneLevelRecusion() basically destroy any pretense of that code possibly being reusable for some

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-08-02 Thread Robert Haas
On Mon, Aug 2, 2010 at 2:56 PM, Yeb Havinga yebhavi...@gmail.com wrote: I do not completely understand what you mean with the destruction of reusability of ATOneLevelRecursion, currently only called by ATPrepAddColumn - in the patch it is documented in the definition of relVisited that is it

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-31 Thread Robert Haas
On Fri, Jul 30, 2010 at 4:38 PM, Yeb Havinga yebhavi...@gmail.com wrote: I'm looking at ATPrepAddColumn right now, where there is actually some comments about getting the right attinhcount in the case of multiple inherited children, but it's the first time I'm looking at this part of

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Robert Haas
On Thu, Jul 29, 2010 at 6:57 AM, Henk Enting h.d.ent...@mgrid.net wrote: We ran into a problem on 9.0beta3 with check constraints using table inheritance in a multi-level hierarchy with multiple inheritance. A test script is provided below and a proposed patch is attached to this email.

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Thu, Jul 29, 2010 at 6:57 AM, Henk Enting h.d.ent...@mgrid.net wrote: We ran into a problem on 9.0beta3 with check constraints using table inheritance in a multi-level hierarchy with multiple inheritance. Thanks for the report. This bug also

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Robert Haas
On Fri, Jul 30, 2010 at 10:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jul 29, 2010 at 6:57 AM, Henk Enting h.d.ent...@mgrid.net wrote: We ran into a problem on 9.0beta3 with check constraints using table inheritance in a multi-level hierarchy

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Fri, Jul 30, 2010 at 10:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: The original design idea was that coninhcount/conislocal would act exactly like attinhcount/attislocal do for multiply-inherited columns. Where did we fail to copy that logic? We

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Robert Haas
On Fri, Jul 30, 2010 at 10:23 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jul 30, 2010 at 10:11 AM, Tom Lane t...@sss.pgh.pa.us wrote: The original design idea was that coninhcount/conislocal would act exactly like attinhcount/attislocal do for

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Fri, Jul 30, 2010 at 10:23 AM, Tom Lane t...@sss.pgh.pa.us wrote: Uh, full stop there.  If you think that the multiply-inherited column logic is wrong, it's you that is mistaken --- or at least you're going to have to do a lot more than just assert

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Yeb Havinga
Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: Since the output in the previous email apparently wasn't sufficient for you to understand what the problem is, here it is in more detail. ... Adding a column to the toplevel parent of the inheritance hierarchy and then dropping it again

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Robert Haas
On Fri, Jul 30, 2010 at 10:46 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jul 30, 2010 at 10:23 AM, Tom Lane t...@sss.pgh.pa.us wrote: Uh, full stop there.  If you think that the multiply-inherited column logic is wrong, it's you that is mistaken

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Tom Lane
Yeb Havinga yebhavi...@gmail.com writes: Regard the following lattice (direction from top to bottom): 1 |\ 2 3 \|\ 4 5 \| 6 When adding a constraint to 1, the proper coninhcount for that constraint on relation 6 is 2. But the code currently counts to 3, since 6 is reached

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Robert Haas
On Fri, Jul 30, 2010 at 11:39 AM, Tom Lane t...@sss.pgh.pa.us wrote: Yeb Havinga yebhavi...@gmail.com writes: Regard the following lattice (direction from top to bottom): 1 |\ 2 3  \|\   4 5    \|     6 When adding a constraint to 1, the proper coninhcount for that constraint on

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Robert Haas
On Fri, Jul 30, 2010 at 11:35 AM, Robert Haas robertmh...@gmail.com wrote: In the case of coninhcount, I believe that the fix actually is quite simple, although of course it's possible that I'm missing something. Currently, ATAddConstraint() first calls AddRelationNewConstraints() to add the

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Yeb Havinga
Robert Haas wrote: It seems to me that it should only recurse to its children if the constraint was really added, rather than merged into an existing constraint, because if it was merged into an existing constraint, then the children already have it. Yes. (then the children already have it -

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Yeb Havinga
Yeb Havinga wrote: Robert Haas wrote: This still leaves open the question of what to do about the similar case involving attributes: That problem looks significantly more difficult to solve, though. I'm looking at ATPrepAddColumn right now, where there is actually some comments about

Re: [HACKERS] patch for check constraints using multiple inheritance

2010-07-30 Thread Alex Hunsaker
On Fri, Jul 30, 2010 at 10:22, Robert Haas robertmh...@gmail.com wrote: OK, it looks like level_2_parent is actually irrelevant to this issue.  So here's a slightly simplified test case: DROP SCHEMA IF EXISTS test_inheritance CASCADE; CREATE SCHEMA test_inheritance; SET search_path TO

[HACKERS] patch for check constraints using multiple inheritance

2010-07-29 Thread Henk Enting
Hi, We ran into a problem on 9.0beta3 with check constraints using table inheritance in a multi-level hierarchy with multiple inheritance. A test script is provided below and a proposed patch is attached to this email. Regards, Henk Enting, Yeb Havinga MGRID B.V. http://www.mgrid.net /*