2010/2/1 KaiGai Kohei kai...@ak.jp.nec.com:
I'm making a general statement - if something is BROKEN (like the
rename case we just dealt with), we should look at fixing it. If it's
just something that could be cleaned up or done more nicely, we should
leave it alone for now.
OK, Please
(2010/02/02 23:50), Robert Haas wrote:
2010/2/1 KaiGai Koheikai...@ak.jp.nec.com:
I'm making a general statement - if something is BROKEN (like the
rename case we just dealt with), we should look at fixing it. If it's
just something that could be cleaned up or done more nicely, we should
2010/1/31 KaiGai Kohei kai...@ak.jp.nec.com:
The attached patch modified find_all_inheritors() to return the list of
expected inhcount, if List * pointer is given. And, it focuses on only
the bugs in renameatt() case.
I have cleaned up and simplified this patch. Attached is the version
I
Robert Haas robertmh...@gmail.com writes:
I have cleaned up and simplified this patch. Attached is the version
I intend to commit. Changes:
Minor suggestions:
I think the names like rel_parents would read better as
rel_numparents etc. As-is, the reader could be forgiven for expecting
that
On Mon, Feb 1, 2010 at 1:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
I have cleaned up and simplified this patch. Attached is the version
I intend to commit. Changes:
Minor suggestions:
I think the names like rel_parents would read better as
On Mon, Feb 1, 2010 at 1:40 PM, Robert Haas robertmh...@gmail.com wrote:
Looks sane otherwise.
Thanks for the review.
Oh, one other thing. Should we backpatch this, or just apply to HEAD?
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to
Robert Haas robertmh...@gmail.com writes:
Oh, one other thing. Should we backpatch this, or just apply to HEAD?
Just HEAD imo. Without any complaints from the field, I don't think
it's worth taking any risks for. It's not like multiple inheritance
is heavily used ...
On Mon, Feb 1, 2010 at 2:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
Oh, one other thing. Should we backpatch this, or just apply to HEAD?
Just HEAD imo. Without any complaints from the field, I don't think
it's worth taking any risks for. It's not
(2010/02/02 3:01), Robert Haas wrote:
2010/1/31 KaiGai Koheikai...@ak.jp.nec.com:
The attached patch modified find_all_inheritors() to return the list of
expected inhcount, if List * pointer is given. And, it focuses on only
the bugs in renameatt() case.
I have cleaned up and simplified
(2010/02/02 9:48), KaiGai Kohei wrote:
Also, the ALTER COLUMN TYPE case should be also fixed in the 9.1 release
(or 9.0.1?).
If the fix is something we could commit for 9.0.1, then we ought to do
it now before 9.0 is released. If you want to submit a follow-on
patch to address ALTER COLUMN
KaiGai Kohei kai...@ak.jp.nec.com writes:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
not only ATPrepAlterColumnType(), according to what I mentioned above.
What exactly do you claim is wrong with the ADD COLUMN case?
regards, tom lane
(2010/02/02 11:09), Tom Lane wrote:
KaiGai Koheikai...@ak.jp.nec.com writes:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
not only ATPrepAlterColumnType(), according to what I mentioned above.
What exactly do you claim is wrong with the ADD COLUMN case?
ADD
2010/2/1 KaiGai Kohei kai...@ak.jp.nec.com:
(2010/02/02 11:09), Tom Lane wrote:
KaiGai Koheikai...@ak.jp.nec.com writes:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
not only ATPrepAlterColumnType(), according to what I mentioned above.
What exactly do you
KaiGai Kohei kai...@ak.jp.nec.com writes:
(2010/02/02 11:09), Tom Lane wrote:
KaiGai Koheikai...@ak.jp.nec.com writes:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn() code,
not only ATPrepAlterColumnType(), according to what I mentioned above.
What exactly do you
(2010/02/02 11:31), Robert Haas wrote:
2010/2/1 KaiGai Koheikai...@ak.jp.nec.com:
(2010/02/02 11:09), Tom Lane wrote:
KaiGai Koheikai...@ak.jp.nec.comwrites:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn()
code,
not only ATPrepAlterColumnType(), according to what
2010/2/1 KaiGai Kohei kai...@ak.jp.nec.com:
(2010/02/02 11:31), Robert Haas wrote:
2010/2/1 KaiGai Koheikai...@ak.jp.nec.com:
(2010/02/02 11:09), Tom Lane wrote:
KaiGai Koheikai...@ak.jp.nec.com writes:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn()
code,
not
(2010/02/02 11:44), Robert Haas wrote:
2010/2/1 KaiGai Koheikai...@ak.jp.nec.com:
(2010/02/02 11:31), Robert Haas wrote:
2010/2/1 KaiGai Koheikai...@ak.jp.nec.com:
(2010/02/02 11:09), Tom Lane wrote:
KaiGai Koheikai...@ak.jp.nec.com writes:
The attached one also clean up
(2010/01/30 3:36), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 9:58), KaiGai Kohei wrote:
(2010/01/29 9:29), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
Hmm,
(2010/02/01 8:41), KaiGai Kohei wrote:
(2010/01/30 3:36), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 9:58), KaiGai Kohei wrote:
(2010/01/29 9:29), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27
2010/1/28 KaiGai Kohei kai...@ak.jp.nec.com:
(2010/01/29 9:58), KaiGai Kohei wrote:
(2010/01/29 9:29), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
Hmm, indeed, this logic (V3/V5) is busted.
2010/1/27 KaiGai Kohei kai...@ak.jp.nec.com:
Hmm, indeed, this logic (V3/V5) is busted.
The idea of V4 patch can also handle this case correctly, although it
is lesser in performance.
I wonder whether it is really unacceptable cost in performance, or not.
Basically, I assume ALTER TABLE
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
Hmm, indeed, this logic (V3/V5) is busted.
The idea of V4 patch can also handle this case correctly, although it
is lesser in performance.
I wonder whether it is really unacceptable cost in performance, or not.
2010/1/28 KaiGai Kohei kai...@ak.jp.nec.com:
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
Hmm, indeed, this logic (V3/V5) is busted.
The idea of V4 patch can also handle this case correctly, although it
is lesser in performance.
I wonder whether it is
(2010/01/29 9:29), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
Hmm, indeed, this logic (V3/V5) is busted.
The idea of V4 patch can also handle this case correctly, although it
is lesser in
(2010/01/29 9:58), KaiGai Kohei wrote:
(2010/01/29 9:29), Robert Haas wrote:
2010/1/28 KaiGai Koheikai...@ak.jp.nec.com:
(2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
Hmm, indeed, this logic (V3/V5) is busted.
The idea of V4 patch can also handle this
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
-
2010/1/27 KaiGai Kohei kai...@ak.jp.nec.com:
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().
I think I was clear about what the next step was for this patch in my
(2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
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().
I think I was clear about what the
On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
(2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
The attached patch is revised one based on the V3 approach.
The only difference from V3 is that it also applies checks on the
--On 27. Januar 2010 15:42:45 -0500 Robert Haas robertmh...@gmail.com
wrote:
Bernd (or anyone), feel free to take a look in parallel. More eyes
would be helpful...
I've planned to look at this tomorrow when i'm back in office.
--
Thanks
Bernd
--
Sent via pgsql-hackers mailing
On Wed, Jan 27, 2010 at 3:42 PM, Robert Haas robertmh...@gmail.com wrote:
On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
(2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
The attached patch is revised one based on the V3 approach.
(2010/01/28 5:42), Robert Haas wrote:
On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Koheikai...@kaigai.gr.jp wrote:
(2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
The attached patch is revised one based on the V3 approach.
The only difference from V3 is that
(2010/01/28 6:58), Robert Haas wrote:
On Wed, Jan 27, 2010 at 3:42 PM, Robert Haasrobertmh...@gmail.com wrote:
On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Koheikai...@kaigai.gr.jp wrote:
(2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Koheikai...@ak.jp.nec.com:
The attached patch is
2010/1/25 KaiGai Kohei kai...@ak.jp.nec.com:
Or, are you saying to test diamond-inheritance cases?
Please go back and read the test case that I already proposed.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
--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`;
(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);
--On 23. Januar 2010 22:29:23 -0500 Robert Haas robertmh...@gmail.com
wrote:
I don't think this is ready for committer, becauseTom previously
objected to the approach taken by this patch here, and no one has
answered his objections:
On Sat, Jan 23, 2010 at 10:48 PM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
(2010/01/24 12:29), Robert Haas wrote:
I don't think this is ready for committer, becauseTom previously
objected to the approach taken by this patch here, and no one has
answered his objections:
On Sun, Jan 24, 2010 at 8:36 AM, Robert Haas robertmh...@gmail.com wrote:
On Sat, Jan 23, 2010 at 10:48 PM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
(2010/01/24 12:29), Robert Haas wrote:
I don't think this is ready for committer, becauseTom previously
objected to the approach taken by this
--On 24. Januar 2010 08:37:13 -0500 Robert Haas robertmh...@gmail.com
wrote:
I agree - the requirements here are much looser than for, say, SELECT
or UPDATE. But it still has to not suck.
Yeah, i think the meaning of suck can be much weakier than for a DML
command. However, if it would
Bernd Helmle maili...@oopsware.de writes:
--On 24. Januar 2010 08:37:13 -0500 Robert Haas robertmh...@gmail.com
wrote:
I think the problem case here might be something like this...
Did that with a crude pl/pgsql script, and got the following numbers:
I think my concern about the original
--On 24. Januar 2010 13:23:14 -0500 Tom Lane t...@sss.pgh.pa.us wrote:
I think my concern about the original proposal was that the time to
perform an ALTER RENAME would increase with the number of tables in the
database, even if they were entirely unrelated to the one you're trying
to rename.
--On 24. Januar 2010 19:45:33 +0100 Bernd Helmle maili...@oopsware.de
wrote:
I don't see where this should be related to the number of tables not
part of the inheritance tree (or inheritance at all).
To answer that myself: it seems get_attname() introduces the overhead here
(forgot
On Sun, Jan 24, 2010 at 2:01 PM, Bernd Helmle maili...@oopsware.de wrote:
--On 24. Januar 2010 19:45:33 +0100 Bernd Helmle maili...@oopsware.de
wrote:
I don't see where this should be related to the number of tables not
part of the inheritance tree (or inheritance at all).
To answer that
(2010/01/25 4:01), Bernd Helmle wrote:
--On 24. Januar 2010 19:45:33 +0100 Bernd Helmle maili...@oopsware.de
wrote:
I don't see where this should be related to the number of tables not
part of the inheritance tree (or inheritance at all).
To answer that myself: it seems get_attname()
(2010/01/25 8:45), KaiGai Kohei wrote:
(2010/01/25 4:01), Bernd Helmle wrote:
--On 24. Januar 2010 19:45:33 +0100 Bernd Helmlemaili...@oopsware.de
wrote:
I don't see where this should be related to the number of tables not
part of the inheritance tree (or inheritance at all).
To answer
2010/1/24 KaiGai Kohei kai...@ak.jp.nec.com:
It seems to me the result is different from Bernd's report.
Well, you tested something different, so you got a different answer.
Your case doesn't have any multiple inheritance.
...Robert
--
Sent via pgsql-hackers mailing list
(2010/01/25 14:08), Robert Haas wrote:
2010/1/24 KaiGai Koheikai...@ak.jp.nec.com:
It seems to me the result is different from Bernd's report.
Well, you tested something different, so you got a different answer.
Your case doesn't have any multiple inheritance.
If it tries ALTER TABLE xxx
--On 14. Januar 2010 16:04:17 +0900 KaiGai Kohei kai...@ak.jp.nec.com
wrote:
This patch adds:
List *find_column_origin(Oid relOid, const char *colName)
It returns the list of relation OIDs which originally defines the given
column. In most cases, it returns a list with an element. But,
On Sat, Jan 23, 2010 at 1:45 PM, Bernd Helmle maili...@oopsware.de wrote:
--On 14. Januar 2010 16:04:17 +0900 KaiGai Kohei kai...@ak.jp.nec.com
wrote:
This patch adds:
List *find_column_origin(Oid relOid, const char *colName)
It returns the list of relation OIDs which originally defines
(2010/01/24 12:29), Robert Haas wrote:
On Sat, Jan 23, 2010 at 1:45 PM, Bernd Helmlemaili...@oopsware.de wrote:
--On 14. Januar 2010 16:04:17 +0900 KaiGai Koheikai...@ak.jp.nec.com
wrote:
This patch adds:
List *find_column_origin(Oid relOid, const char *colName)
It returns the list of
The similar matter can be reproduced with ALTER TABLE ... TYPE statement,
not only RENAME TO option.
postgres=# CREATE TABLE t1 (a int);
CREATE TABLE
postgres=# CREATE TABLE s1 (a int);
CREATE TABLE
postgres=# CREATE TABLE ts (b int) inherits (t1, s1);
NOTICE: merging multiple
The attached patch fixes bugs when we try to rename (and change type) on
a column inherited from the different origin and merged.
This patch adds:
List *find_column_origin(Oid relOid, const char *colName)
It returns the list of relation OIDs which originally defines the given
column. In most
The method I suggested would allow the
necessary information to be extracted during the initial search for
child tables, which we have to do anyway.
find_all_inheritors() returns a clean list which does not contain
duplicated OID of the inherited relation, so it seems to me we need
to
On Sun, Jan 3, 2010 at 11:18 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
2010/1/3 KaiGai Kohei kai...@ak.jp.nec.com:
if (number_of_attribute_origin(myrelid, oldattname) 1)
ereport(ERROR, ...);
Am I missing something?
That sounds about right to
On Sat, Jan 2, 2010 at 2:32 PM, Tom Lane t...@sss.pgh.pa.us wrote:
KaiGai Kohei kai...@kaigai.gr.jp writes:
(2009/12/30 10:38), Robert Haas wrote:
No longer applies. Can you rebase?
The attached patch is the rebased revision.
I'm not really impressed with this patch, because it will reject
Robert Haas robertmh...@gmail.com writes:
Upthread you appeared to be endorsing what KaiGai has implemented here:
http://archives.postgresql.org/pgsql-hackers/2009-11/msg00147.php
No, I said that forbidding conflicting renames would be a good solution.
I did not endorse any specific means of
On Jan 3, 2010, at 12:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
In practice the reasonable engineering alternatives may just be to do
what KaiGai's patch does, or to do nothing. In that case I think a
good
argument can be made for the latter. Nobody has ever complained about
this from the
(2010/01/04 4:06), Robert Haas wrote:
On Jan 3, 2010, at 12:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
In practice the reasonable engineering alternatives may just be to do
what KaiGai's patch does, or to do nothing. In that case I think a good
argument can be made for the latter. Nobody has
2010/1/3 KaiGai Kohei kai...@ak.jp.nec.com:
(2010/01/04 4:06), Robert Haas wrote:
On Jan 3, 2010, at 12:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
In practice the reasonable engineering alternatives may just be to do
what KaiGai's patch does, or to do nothing. In that case I think a good
Robert Haas robertmh...@gmail.com writes:
2010/1/3 KaiGai Kohei kai...@ak.jp.nec.com:
if (number_of_attribute_origin(myrelid, oldattname) 1)
ereport(ERROR, ...);
Am I missing something?
That sounds about right to me,
It looks remarkably inefficient to me. Do you propose to search
(2010/01/04 13:18), Tom Lane wrote:
Robert Haasrobertmh...@gmail.com writes:
2010/1/3 KaiGai Koheikai...@ak.jp.nec.com:
�if (number_of_attribute_origin(myrelid, oldattname) 1)
� � �ereport(ERROR, ...);
Am I missing something?
That sounds about right to me,
It looks remarkably
(2009/12/30 10:38), Robert Haas wrote:
2009/12/16 KaiGai Koheikai...@ak.jp.nec.com:
It is a patch for the matter which I reported before.
When a column is inherited from multiple relations, ALTER TABLE with
RENAME TO option is problematic.
This patch fixes the matter. In correctly, it prevent
KaiGai Kohei kai...@kaigai.gr.jp writes:
(2009/12/30 10:38), Robert Haas wrote:
No longer applies. Can you rebase?
The attached patch is the rebased revision.
I'm not really impressed with this patch, because it will reject
perfectly legitimate multiple-inheritance cases (ie, cases where
2009/12/16 KaiGai Kohei kai...@ak.jp.nec.com:
It is a patch for the matter which I reported before.
When a column is inherited from multiple relations, ALTER TABLE with
RENAME TO option is problematic.
This patch fixes the matter. In correctly, it prevent to rename columns
inherited from
It is a patch for the matter which I reported before.
When a column is inherited from multiple relations, ALTER TABLE with
RENAME TO option is problematic.
This patch fixes the matter. In correctly, it prevent to rename columns
inherited from multiple relations and merged.
Also see the past
KaiGai Kohei wrote:
postgres=# SELECT * FROM t2;
ERROR: could not find inherited attribute b of relation t3
Because t3.b is also inherited from the t2, but ALTER TABLE does not
care about multiple inherited columns well.
I think we should not allow to rename a column with
2009/11/4 Alvaro Herrera alvhe...@commandprompt.com:
KaiGai Kohei wrote:
postgres=# SELECT * FROM t2;
ERROR: could not find inherited attribute b of relation t3
Because t3.b is also inherited from the t2, but ALTER TABLE does not
care about multiple inherited columns well.
I think we
Thom Brown thombr...@gmail.com writes:
2009/11/4 Alvaro Herrera alvhe...@commandprompt.com:
KaiGai Kohei wrote:
I think we should not allow to rename a column with attinhcount 1.
I think we should fix ALTER TABLE to cope with multiple inheritance.
I'd be interested to see how this should
Tom Lane wrote:
Thom Brown thombr...@gmail.com writes:
2009/11/4 Alvaro Herrera alvhe...@commandprompt.com:
KaiGai Kohei wrote:
I think we should not allow to rename a column with attinhcount 1.
I think we should fix ALTER TABLE to cope with multiple inheritance.
I'd be interested to
70 matches
Mail list logo
