Prev: knngist - 0.8
Next: [HACKERS] reducing NUMERIC size for 9.1, take two
From: Yeb Havinga on 2 Aug 2010 09:20
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,
i.e. it is a multiple inheriting child. The other solutions I could
think of are more intrusive (i.e. definitionin ATController and passing
as parameter).

The attached patch uses the globally defined list. After ATPrepCmd the
list pointer is reset to NIL, the list not freed since the allocs are
done in a memory context soon to be deleted (PortalHeapMemory). It
passes regression as well as the script below.

regards,
Yeb Havinga


DROP SCHEMA IF EXISTS test_inheritance CASCADE;
CREATE SCHEMA test_inheritance;
SET search_path TO test_inheritance;

CREATE TABLE top (i int);
CREATE TABLE mid1 () INHERITS (top);
CREATE TABLE mid2 () INHERITS (top);
CREATE TABLE bottom () INHERITS (mid1, mid2);
CREATE TABLE basement () INHERITS (bottom);

ALTER TABLE top
ADD COLUMN a_table_column integer,
ADD COLUMN a_table_column2 integer;

ALTER TABLE top
ADD COLUMN a_table_column3 integer;

ALTER TABLE top
ADD CONSTRAINT a_check_constraint CHECK (i IN (0,1)),
ADD CONSTRAINT a_check_constraint2 CHECK (i IN (0,1));

ALTER TABLE top
ADD CONSTRAINT a_check_constraint3 CHECK (i IN (0,1));

SELECT t.oid, t.relname, a.attinhcount
FROM pg_class t
JOIN pg_attribute a ON (a.attrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance' AND a.attname LIKE 'a_table_column%'
ORDER BY oid;

SELECT t.oid, t.relname, c.coninhcount
FROM pg_class t
JOIN pg_constraint c ON (c.conrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance'
ORDER BY oid;




From: Yeb Havinga on 3 Aug 2010 11:34
Robert Haas wrote:
> On Mon, Aug 2, 2010 at 2:56 PM, Yeb Havinga <yebhavinga(a)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"
> behavior, which might not be true for every command type.
It is actually "visit each table only once for each distinct parent".
Looking at the command types for ALTER TABLE, I see none where this
behaviour would be incorrect.

That put aside, the top1/top2 example is interesting in the sense that
it reveals other problems besides the wrong attinhcount at the basement.
For an example see the script below. 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 code does a lot of trouble for to recognize. This
means that if some definition is altered on A.x, only C.x is updated and
B.x not touched. IMO this is wrong and either a multiple inheritance
structure like this should be prohibited, since the user did not
explicitly declare that A.x and B.x 'are the same' (by e.g. defining a
relation D.x and have A and B inherit from that), or the code should
update parents of relations when the childs are updated.

The difficulty is in exactly specifying the 'is the same' as relation,
i.e. under what conditions are columns A.x and B.x allowed to be merged
to C.x. In the regression test there's only a small amount of tests, but
one of them shows that the 'is the same' as relation does not mean
everything is the same, as it shows that default values may differ.

regards,
Yeb Havinga


DROP SCHEMA IF EXISTS test_inheritance CASCADE;
CREATE SCHEMA test_inheritance;
SET search_path TO test_inheritance;

CREATE TABLE top1 (i int);
CREATE TABLE top2 (i int);
CREATE TABLE bottom () INHERITS (top1, top2);
CREATE TABLE basement () INHERITS (bottom);

ALTER TABLE top1 ADD COLUMN a_table_column INTEGER;
ALTER TABLE top2 ADD COLUMN a_table_column INTEGER;

SELECT t.oid, t.relname, a.attinhcount, a.attname
FROM pg_class t
JOIN pg_attribute a ON (a.attrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance' AND a.attname LIKE '%table_column%'
ORDER BY oid;

ALTER TABLE top1 RENAME COLUMN a_table_column TO another_table_column;

SELECT t.oid, t.relname, a.attinhcount, a.attname
FROM pg_class t
JOIN pg_attribute a ON (a.attrelid = t.oid)
JOIN pg_namespace n ON (t.relnamespace = n.oid)
WHERE n.nspname = 'test_inheritance' AND a.attname LIKE '%table_column%'
ORDER BY oid;

ALTER TABLE top2 RENAME COLUMN a_table_column TO another_table_column;


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Yeb Havinga on 3 Aug 2010 15:05
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 code
> does a lot of trouble for to recognize. This means that if some
> definition is altered on A.x, only C.x is updated and B.x not touched.
> IMO this is wrong and either a multiple inheritance structure like
> this should be prohibited, since the user did not explicitly declare
> that A.x and B.x 'are the same' (by e.g. defining a relation D.x and
> have A and B inherit from that), or the code should update parents of
> relations when the childs are updated.
Thinking about this a bit more, the name 'is the same as' is a bit
confusing, since that relation might not be commutative. C.x 'inherits
properties from' A.x, or C.x 'is defined by' A.x are perhaps better
names, that reflect that the converse might not hold. OTOH, what does
C.x 'inherits (all) properties from' A.x mean? If it means that for all
properties P, P(C.x) iff P(A.x), then C.x = A.x commutatively and by
similar reasoning A.x = B.x.

> ALTER TABLE top1 RENAME COLUMN a_table_column TO another_table_column;
When looking for previous discussions that was referred to upthread, the
first thing I found was this recent thread about the exactly the same
problem http://archives.postgresql.org/pgsql-hackers/2010-01/msg03117.php

Sorry for the double post, however the previous discussion postponed
work to .. now, so maybe there is some value in first trying to specify
exactly what 'inherits' means, and derive consequences for code
behaviour from that.

regards,
Yeb Havinga


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

First  |  Prev  | 
Pages: 1 2 3 4 5
Prev: knngist - 0.8
Next: [HACKERS] reducing NUMERIC size for 9.1, take two