Renaming a table to an array's autogenerated name
In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo
when an array type of that name already existed would make the array
type change its name to get out of the way. But it missed a trick in
that renaming a table would still cause a conflict.
Steps to reproduce:
postgres=# create table foo (id int);
CREATE TABLE
postgres=# create table bar (id int);
CREATE TABLE
postgres=# alter table bar rename to _foo;
ERROR: type "_foo" already exists
Attached is a patch that fixes this bug.
One interesting thing to note however, is that if you rename a table to
its own array's name (which was my case when I found this bug), the new
array name will be ___foo instead of just __foo. I don't know if it's
worth worrying about that.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
rename-_foo-v01.patchtext/x-patch; name=rename-_foo-v01.patchDownload+35-3
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo
when an array type of that name already existed would make the array
type change its name to get out of the way. But it missed a trick in
that renaming a table would still cause a conflict.
Good catch.
One interesting thing to note however, is that if you rename a table to
its own array's name (which was my case when I found this bug), the new
array name will be ___foo instead of just __foo. I don't know if it's
worth worrying about that.
This seemed weird to me. Stepping through the logic, I see that what's
happening is that the moveArrayTypeName call you added renames the
array type from "_foo" to "__foo", and then at the bottom of
RenameTypeInternal we call makeArrayTypeName/RenameTypeInternal again
on that array type. makeArrayTypeName sees that "__foo" exists and
assumes the name is in use, even though it's really this same type and
no more renaming is needed. So it ends up picking the next choice
"___foo".
After some experimentation, I came up with the attached, which simply
skips the "recursive" step if it would apply to the same array type we
already moved.
I added some queries to the regression test case to show exactly what
happens to the array type names, and in that, you can see that the
behavior for the "normal" case with distinct array types is that neither
array type ends up with a name that is just the unsurprising case of
an underscore followed by its element type's name; they *both* have an
extra underscore compared to that. Maybe that's okay. We could possibly
rejigger the order of renaming so that one of them ends with an
unsurprising name, but I failed to make that happen without considerably
more surgery.
regards, tom lane
Attachments:
rename-_foo-v02.patchtext/x-patch; charset=us-ascii; name=rename-_foo-v02.patchDownload+107-37
On 05/25/2017 05:24 PM, Tom Lane wrote:
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo
when an array type of that name already existed would make the array
type change its name to get out of the way. But it missed a trick in
that renaming a table would still cause a conflict.Good catch.
One interesting thing to note however, is that if you rename a table to
its own array's name (which was my case when I found this bug), the new
array name will be ___foo instead of just __foo. I don't know if it's
worth worrying about that.After some experimentation, I came up with the attached, which simply
skips the "recursive" step if it would apply to the same array type we
already moved.
This looks good to me.
I added some queries to the regression test case to show exactly what
happens to the array type names, and in that, you can see that the
behavior for the "normal" case with distinct array types is that neither
array type ends up with a name that is just the unsurprising case of
an underscore followed by its element type's name; they *both* have an
extra underscore compared to that. Maybe that's okay. We could possibly
rejigger the order of renaming so that one of them ends with an
unsurprising name, but I failed to make that happen without considerably
more surgery.
I don't think this really matters to anyone in practice, so I'm fine
with it.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
On 05/25/2017 05:24 PM, Tom Lane wrote:
After some experimentation, I came up with the attached, which simply
skips the "recursive" step if it would apply to the same array type we
already moved.
This looks good to me.
Pushed, thanks for reviewing.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/26/2017 03:20 PM, Tom Lane wrote:
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
On 05/25/2017 05:24 PM, Tom Lane wrote:
After some experimentation, I came up with the attached, which simply
skips the "recursive" step if it would apply to the same array type we
already moved.This looks good to me.
Pushed, thanks for reviewing.
Thanks!
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers