(2010/02/02 23:50), Robert Haas wrote:
> 2010/2/1 KaiGai Kohei:
>>> 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 al
2010/2/1 KaiGai Kohei :
>> 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 forget the secon
(2010/02/02 11:44), Robert Haas wrote:
> 2010/2/1 KaiGai Kohei:
>> (2010/02/02 11:31), Robert Haas wrote:
>>> 2010/2/1 KaiGai Kohei:
(2010/02/02 11:09), Tom Lane wrote:
> KaiGai Kohei writes:
>> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn()
>> code,
>
2010/2/1 KaiGai Kohei :
> (2010/02/02 11:31), Robert Haas wrote:
>> 2010/2/1 KaiGai Kohei:
>>> (2010/02/02 11:09), Tom Lane wrote:
KaiGai Kohei writes:
> The attached one also clean up ATPrepAddColumn() and ATExecAddColumn()
> code,
> not only ATPrepAlterColumnType(), according
(2010/02/02 11:31), Robert Haas wrote:
> 2010/2/1 KaiGai Kohei:
>> (2010/02/02 11:09), Tom Lane wrote:
>>> KaiGai Koheiwrites:
The attached one also clean up ATPrepAddColumn() and ATExecAddColumn()
code,
not only ATPrepAlterColumnType(), according to what I mentioned above.
>>>
KaiGai Kohei writes:
> (2010/02/02 11:09), Tom Lane wrote:
>> KaiGai Kohei 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 COL
2010/2/1 KaiGai Kohei :
> (2010/02/02 11:09), Tom Lane wrote:
>> KaiGai Kohei 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 CO
(2010/02/02 11:09), Tom Lane wrote:
> KaiGai Kohei 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 COLUMN case wo
KaiGai Kohei 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
--
Sent via pgsql-
(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
(2010/02/02 3:01), Robert Haas wrote:
> 2010/1/31 KaiGai Kohei:
>> 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. At
On Mon, Feb 1, 2010 at 2:03 PM, Tom Lane wrote:
> Robert Haas 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 heav
Robert Haas 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 ...
regards, tom lane
--
On Mon, Feb 1, 2010 at 1:40 PM, Robert Haas 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 your subscription:
ht
On Mon, Feb 1, 2010 at 1:31 PM, Tom Lane wrote:
> Robert Haas 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
Robert Haas 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 this will be a li
2010/1/31 KaiGai Kohei :
> 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 intend to commit. Cha
(2010/02/01 8:41), KaiGai Kohei wrote:
> (2010/01/30 3:36), Robert Haas wrote:
>> 2010/1/28 KaiGai Kohei:
>>> (2010/01/29 9:58), KaiGai Kohei wrote:
(2010/01/29 9:29), Robert Haas wrote:
> 2010/1/28 KaiGai Kohei:
>> (2010/01/29 0:46), Robert Haas wrote:
>>> 2010/1/27 KaiGai Kohei:
(2010/01/30 3:36), Robert Haas wrote:
> 2010/1/28 KaiGai Kohei:
>> (2010/01/29 9:58), KaiGai Kohei wrote:
>>> (2010/01/29 9:29), Robert Haas wrote:
2010/1/28 KaiGai Kohei:
> (2010/01/29 0:46), Robert Haas wrote:
>> 2010/1/27 KaiGai Kohei:
>>> Hmm, indeed, this logic (V3/V5) is bust
2010/1/28 KaiGai Kohei :
> (2010/01/29 9:58), KaiGai Kohei wrote:
>> (2010/01/29 9:29), Robert Haas wrote:
>>> 2010/1/28 KaiGai Kohei:
(2010/01/29 0:46), Robert Haas wrote:
> 2010/1/27 KaiGai Kohei:
>> Hmm, indeed, this logic (V3/V5) is busted.
>>
>> The idea of V4 patch can al
(2010/01/29 9:58), KaiGai Kohei wrote:
> (2010/01/29 9:29), Robert Haas wrote:
>> 2010/1/28 KaiGai Kohei:
>>> (2010/01/29 0:46), Robert Haas wrote:
2010/1/27 KaiGai Kohei:
> Hmm, indeed, this logic (V3/V5) is busted.
>
> The idea of V4 patch can also handle this case correctly, alt
(2010/01/29 9:29), Robert Haas wrote:
> 2010/1/28 KaiGai Kohei:
>> (2010/01/29 0:46), Robert Haas wrote:
>>> 2010/1/27 KaiGai Kohei:
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
2010/1/28 KaiGai Kohei :
> (2010/01/29 0:46), Robert Haas wrote:
>> 2010/1/27 KaiGai Kohei:
>>> 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
(2010/01/29 0:46), Robert Haas wrote:
> 2010/1/27 KaiGai Kohei:
>> 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.
>> Basica
2010/1/27 KaiGai Kohei :
> 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 RENAME/TYPE is not
(2010/01/28 6:58), Robert Haas wrote:
> On Wed, Jan 27, 2010 at 3:42 PM, Robert Haas wrote:
>> On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Kohei wrote:
>>> (2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Kohei:
>
> 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 Kohei wrote:
>> (2010/01/27 23:29), Robert Haas wrote:
>>>
>>> 2010/1/27 KaiGai Kohei:
The attached patch is revised one based on the V3 approach.
The only difference from V3 is that it also applies che
On Wed, Jan 27, 2010 at 3:42 PM, Robert Haas wrote:
> On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Kohei wrote:
>> (2010/01/27 23:29), Robert Haas wrote:
>>>
>>> 2010/1/27 KaiGai Kohei:
The attached patch is revised one based on the V3 approach.
The only difference from V3 is that it a
--On 27. Januar 2010 15:42:45 -0500 Robert Haas
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 list (pgsql-hackers@p
On Wed, Jan 27, 2010 at 10:17 AM, KaiGai Kohei wrote:
> (2010/01/27 23:29), Robert Haas wrote:
>>
>> 2010/1/27 KaiGai Kohei:
>>>
>>> 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
(2010/01/27 23:29), Robert Haas wrote:
2010/1/27 KaiGai Kohei:
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 thi
2010/1/27 KaiGai Kohei :
> 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
previous email, but le
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/01/26 1:11), Bernd Helmle wrote:
>
>
> --On 25. Januar 2010 11:39:21 +0900 KaiGai Kohei
> 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 25. Januar 2010 11:39:21 +0900 KaiGai Kohei
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
2010/1/25 KaiGai Kohei :
> 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:
http://www.postgresql.org/mailpre
(2010/01/25 14:08), Robert Haas wrote:
> 2010/1/24 KaiGai Kohei:
>> 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 RENAME TO on any
2010/1/24 KaiGai Kohei :
> 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 (pgsql-hackers@postgresql.org)
To make ch
(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 Helmle
>> 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 t
(2010/01/25 4:01), Bernd Helmle wrote:
>
>
> --On 24. Januar 2010 19:45:33 +0100 Bernd Helmle
> 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() introduce
On Sun, Jan 24, 2010 at 2:01 PM, Bernd Helmle wrote:
> --On 24. Januar 2010 19:45:33 +0100 Bernd Helmle
> 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() introd
--On 24. Januar 2010 19:45:33 +0100 Bernd Helmle
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 about that). Creating add
--On 24. Januar 2010 13:23:14 -0500 Tom Lane 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.
Uhm, find_column
Bernd Helmle writes:
> --On 24. Januar 2010 08:37:13 -0500 Robert Haas
> 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 proposal was that the time to
perform a
--On 24. Januar 2010 08:37:13 -0500 Robert Haas
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 degrade the perform
On Sun, Jan 24, 2010 at 8:36 AM, Robert Haas wrote:
> On Sat, Jan 23, 2010 at 10:48 PM, KaiGai Kohei 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
>>> answ
On Sat, Jan 23, 2010 at 10:48 PM, KaiGai Kohei 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:
>>
>> http://archives.postgresql.org/p
--On 23. Januar 2010 22:29:23 -0500 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:
http://archives.postgresql.org/pgsql-hackers/2010-01/msg00144.php
Ugh, i tho
(2010/01/24 12:29), Robert Haas wrote:
On Sat, Jan 23, 2010 at 1:45 PM, Bernd Helmle wrote:
--On 14. Januar 2010 16:04:17 +0900 KaiGai Kohei
wrote:
This patch adds:
List *find_column_origin(Oid relOid, const char *colName)
It returns the list of relation OIDs which originally defines the g
On Sat, Jan 23, 2010 at 1:45 PM, Bernd Helmle wrote:
> --On 14. Januar 2010 16:04:17 +0900 KaiGai Kohei
> 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 ca
--On 14. Januar 2010 16:04:17 +0900 KaiGai Kohei
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, if the
column is inhe
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 c
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 inheri
On Sun, Jan 3, 2010 at 11:18 PM, Tom Lane wrote:
> Robert Haas writes:
>> 2010/1/3 KaiGai Kohei :
>>> 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
>> 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
(2010/01/04 13:18), Tom Lane wrote:
Robert Haas writes:
2010/1/3 KaiGai Kohei:
�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 the
e
Robert Haas writes:
> 2010/1/3 KaiGai Kohei :
>> 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 the
entire database's inheritan
2010/1/3 KaiGai Kohei :
> (2010/01/04 4:06), Robert Haas wrote:
>> On Jan 3, 2010, at 12:31 PM, Tom Lane 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 lat
(2010/01/04 4:06), Robert Haas wrote:
> On Jan 3, 2010, at 12:31 PM, Tom Lane 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 complaine
On Jan 3, 2010, at 12:31 PM, Tom Lane 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 field AFAIR; but we
Robert Haas 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 testing for such a con
On Sat, Jan 2, 2010 at 2:32 PM, Tom Lane wrote:
> KaiGai Kohei 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 mul
KaiGai Kohei 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 there's
more than o
(2009/12/30 10:38), Robert Haas wrote:
2009/12/16 KaiGai Kohei:
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
inh
2009/12/16 KaiGai Kohei :
> 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
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 discu
Tom Lane wrote:
> Thom Brown writes:
>> 2009/11/4 Alvaro Herrera :
>>> 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 work.
>
>
Thom Brown writes:
> 2009/11/4 Alvaro Herrera :
>> 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 work.
Yeah. I don't think a "fix" i
2009/11/4 Alvaro Herrera :
> 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
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
Is it an expected behavior?
postgres=# CREATE TABLE t1 (a int, b int);
CREATE TABLE
postgres=# CREATE TABLE t2 (b int, c int);
CREATE TABLE
postgres=# CREATE TABLE t3 (d int) inherits (t1, t2);
NOTICE: merging multiple inherited definitions of column "b"
CREATE TABLE
The t3.d is in
71 matches
Mail list logo
