Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Hmm, seems that dblink should call truncate_identifier() for the truncation, to be consistent with truncation of table names etc. Hmmm, we need the same routine with truncate_identifier(), but we hard to use the function because it modifies the input buffer directly. Since all of the name strings in dblink is const char *, I added a bit modified version of the function as truncate_identifier_copy() in the attached v2 patch. I also spotted this in dblink.c: /* first gather the server connstr options */ if (strlen(servername) NAMEDATALEN) foreign_server = GetForeignServerByName(servername, true); I think that's wrong. We normally consistently truncate identifiers at creation and at use, so that if you create an object with a very long name and it's truncated, you can still refer to it with the untruncated name because all such references are truncated too. Absolutely. I re-use the added function for the fix. Regards, --- Takahiro Itagaki NTT Open Source Software Center dblink_63bytes-2010602.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names
On 02/06/10 09:46, Takahiro Itagaki wrote: Heikki Linnakangasheikki.linnakan...@enterprisedb.com wrote: Hmm, seems that dblink should call truncate_identifier() for the truncation, to be consistent with truncation of table names etc. Hmmm, we need the same routine with truncate_identifier(), but we hard to use the function because it modifies the input buffer directly. Since all of the name strings in dblink is const char *, I added a bit modified version of the function as truncate_identifier_copy() in the attached v2 patch. Well, looking at the callers, most if not all of them seem to actually pass a palloc'd copy, allocated by text_to_cstring(). Should we throw a NOTICE like vanilla truncate_identifier() does? I feel it would be better to just call truncate_identifier() than roll your own. Assuming we want the same semantics in dblink, we'll otherwise have to remember to update truncate_identifier_copy() with any changes to truncate_identifier(). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Well, looking at the callers, most if not all of them seem to actually pass a palloc'd copy, allocated by text_to_cstring(). Should we throw a NOTICE like vanilla truncate_identifier() does? I feel it would be better to just call truncate_identifier() than roll your own. Assuming we want the same semantics in dblink, we'll otherwise have to remember to update truncate_identifier_copy() with any changes to truncate_identifier(). That makes sense. Now I use truncate_identifier(warn=true) for the fix. Regards, --- Takahiro Itagaki NTT Open Source Software Center dblink_63bytes-2010-603.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names
On 01/06/10 05:55, Takahiro Itagaki wrote: Takahiro Itagakiitagaki.takah...@oss.ntt.co.jp wrote: Contib/dblink module seems to have a bug in handling connection names in NAMEDATALEN-1 bytes. Here is a patch to fix the bug. I think it comes from wrong usage of snprintf(NAMEDATALEN - 1). It just copies 62 bytes + \0. In addition, it should be safe to use pg_mbcliplen() to truncate extra bytes in connection names because we might return invalid text when a multibyte character is at 62 or 63 bytes. Hmm, seems that dblink should call truncate_identifier() for the truncation, to be consistent with truncation of table names etc. I also spotted this in dblink.c: /* first gather the server connstr options */ if (strlen(servername) NAMEDATALEN) foreign_server = GetForeignServerByName(servername, true); I think that's wrong. We normally consistently truncate identifiers at creation and at use, so that if you create an object with a very long name and it's truncated, you can still refer to it with the untruncated name because all such references are truncated too. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote:
Bug reference: 5487
Logged by: Takahiro Itagaki
Email address: itagaki.takah...@oss.ntt.co.jp
Description:dblink failed with 63 bytes connection names
Details:
Contib/dblink module seems to have a bug in handling
connection names in NAMEDATALEN-1 bytes.
Here is a patch to fix the bug. I think it comes from wrong usage
of snprintf(NAMEDATALEN - 1). It just copies 62 bytes + \0.
In addition, it should be safe to use pg_mbcliplen() to truncate
extra bytes in connection names because we might return invalid
text when a multibyte character is at 62 or 63 bytes.
Note that the fix should be ported to previous versions, too.
It cannot use exiting connections with 63 bytes name
in some cases. For example, we cannot disconnect
such connections. Also, we can reconnect with the
same name and will have two connections with the name.
=# SELECT dblink_connect(repeat('1234567890', 6) || 'ABC',
'host=localhost');
dblink_connect
OK
(1 row)
=# SELECT dblink_get_connections();
dblink_get_connections
---
{123456789012345678901234567890123456789012345678901234567890ABC}
(1 row)
=# SELECT dblink_disconnect(repeat('1234567890', 6) || 'ABC');
ERROR: connection
123456789012345678901234567890123456789012345678901234567890ABC not
available
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
dblink_63bytes.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
