The attached patch is revised one based on the V3 approach. The only difference from V3 is that it also applies checks on the AT_AlterColumnType option, not only renameatt().
The performance was almost same as the V3 case. > * CVS HEAD > 0.828s > 0.828s > 0.833s > 0.829s > 0.838s - ALTER RENAME TO with V5 patch 2.419s 2.418s 2.418s 2.426s I also checked ALTER ... TYPE cases. It is relatively heavy operation than renameatt(), so its affects was relatively smaller. - ALTER ... TYPE with CVS HEAD 28.888s 29.948s 30.738s 30.600s - ALTER ... TYPE with V5 patch 28.067s 28.212s 28.038s 29.497s (2010/01/26 10:10), KaiGai Kohei wrote: > (2010/01/26 1:11), Bernd Helmle wrote: >> >> >> --On 25. Januar 2010 11:39:21 +0900 KaiGai Kohei<kai...@ak.jp.nec.com> >> wrote: >> >>> (echo "CREATE TABLE t (a int);" >>> for i in `seq 0 9`; do >>> echo "CREATE TABLE s$i (b int) INHERITS(t);" >>> for j in `seq 0 9`; do >>> echo "CREATE TABLE v$i$j (c int) INHERITS(s$i);" >>> for k in `seq 0 9`; do >>> echo "CREATE TABLE w$i$j$k (d int) INHERITS(v$i$j);" >>> for l in `seq 0 9`; do >>> echo "CREATE TABLE x$i$j$k$l (e int) INHERITS(w$i$j$k);" >>> done >>> done >>> done >>> done) | psql test >> >> Well, each table inherits one table in your test. In my test, I inherit >> from multiple tables for each table. My script generates the following >> inheritance tree (and wins a price of copy& paste ugliness, see >> attachment): >> >> A1, A2, A3, ..., Am >> B1 INHERITS(A1...A10), B2 INHERITS(A1...A10, B3 INHERITS(A1...A10), ...Bn >> C1 INHERITS(B1...B10), C2 INHERITS(B1...B10), ... Co >> D1 INHERITS(C1...C10), ..., Dp >> >> m = 10 >> n = 10 >> o = 10 >> p = 1000 >> >> Repeating this on my MacBook gives: >> >> ALTER TABLE a1 RENAME COLUMN acol1 TO xyz; >> >> -HEAD: >> >> Time: 382,427 ms >> Time: 375,974 ms >> Time: 385,478 ms >> Time: 371,067 ms >> Time: 410,834 ms >> Time: 386,382 ms >> >> Recent V4 patch: >> >> Time: 6065,673 ms >> Time: 3823,206 ms >> Time: 4037,933 ms >> Time: 3873,029 ms >> Time: 3899,607 ms >> Time: 3963,308 ms > > Hmm... I also could observe similar result in 4 times iteration of > ALTER TABLE with your test_rename.sql. > I agree the recent V4 patch is poor in performance perspective. > > * CVS HEAD > 0.828s > 0.828s > 0.833s > 0.829s > 0.838s > > * Rcent V4 patch: > 10.283s > 10.135s > 10.107s > 10.382s > 10.162s > > * Previous V3 patch: > 2.607s > 2.429s > 2.431s > 2.436s > 2.428s > > The V3 patch is designed to compute an expected inhcount for each relations > to be altered at first, then it shall be compared to pg_attribute.inhcount > to be renamed. > > Basically, its execution cost is same order except for a case when a relation > has diamond inheritance tree. > > The find_all_inheritors() does not check child relations which is already > scanned. However, in this case, we have to check how many times is the child > relation inherited from a common origin. > I guess it is reason of the different between -HEAD and V3. > > For example, if we have the following inheritance tree, > > A2 A5 > / \ \ > A1 A4 > \ / \ > A3 -- A6 > > The find_all_inheritors() checks existence of directly inherited relations > at A1, ... , A6 without any duplications, because this function does not > intend to compute how many times was it inherited. > > The find_all_inheritors_with_inhcount() in V3 patch checks existence of > directly inherited relations, even if the target relation is already checked, > because it also has to return the times to be inherited from a common origin. > In this example, it considers the above structure is same as the following > tree. In this diagram, we can find A4 and A5 twice, and A6 thrice. > > A5 > / > A2 - A4 - A6 > \ > A1 > \ > A3 - A4 - A6 > \ \ > A6 A5 > > Thus, the test_rename.sql was the worst case test for V3 also. > However, I don't think we should keep the bug in the next release. > The CVS HEAD's performance is the result of omission for necessary checks. > > I think we should back to the V3 patch approach, and also reconsider > the logic in ATPrepAlterColumnType(). > > Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kai...@ak.jp.nec.com>
pgsql-fix-inherit-rename.5.patch
Description: application/octect-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
