FAQ

"Takahiro Itagaki" wrote:

Bug reference: 5487
Logged by: Takahiro Itagaki
Email address: itagaki.takahiro@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

Search Discussions

  • Heikki Linnakangas at Jun 1, 2010 at 9:00 am

    On 01/06/10 05:55, Takahiro Itagaki wrote:
    "Takahiro Itagaki"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
  • Takahiro Itagaki at Jun 2, 2010 at 6:46 am

    Heikki Linnakangas 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
  • Heikki Linnakangas at Jun 2, 2010 at 11:17 am

    On 02/06/10 09:46, Takahiro Itagaki wrote:
    Heikki Linnakangaswrote:
    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
  • Takahiro Itagaki at Jun 3, 2010 at 1:07 am

    Heikki Linnakangas 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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJun 1, '10 at 2:54a
activeJun 3, '10 at 1:07a
posts5
users2
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase