postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import - Mailing list pgsql-hackers

From Chao Li
Subject postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import
Date
Msg-id 81D981EB-ECC1-495D-8EAC-5CFB67B2CF77@gmail.com
Whole thread
Responses Re: postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import
List pgsql-hackers
Hi,

I’m testing "[28972b6fc] Add support for importing statistics from remote servers". Functionally, so far so good. I
justfound a piece of suspicious code that might be an oversight in the follow-up commit aa1f93a3387: 
```
/*
 * Compare two RemoteAttributeMappings for sorting.
 */
static int
remattrmap_cmp(const void *v1, const void *v2)
{
    const RemoteAttributeMapping *r1 = v1;
    const RemoteAttributeMapping *r2 = v2;

    return strncmp(r1->remote_attname, r2->remote_attname, NAMEDATALEN);
}
```

This function compares remote attribute names only up to NAMEDATALEN. But aa1f93a3387 changed remote_attname from a
NAMEDATALEN-bytearray to a char pointer, and its commit message explicitly says "the remote column name in particular
couldbe longer than NAMEDATALEN - 1". In that case, remattrmap_cmp() could treat two distinct remote column names as
equalfor sorting, while match_attrmap() later compares the full strings. 

Also, aa1f93a3387’s commit message mentions match_attrmap(), and that function uses strcmp(). So I guess
remattrmap_cmp()should also use strcmp(). 

I don’t have a repro that triggers a bad result. I just want to report and confirm whether that code should be changed.
Imade a local change as the attached diff, and no test is broken with the change. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/





Attachment

pgsql-hackers by date:

Previous
From: Ewan Young
Date:
Subject: Re: Discard ORDER BY/DISTINCT when an ANY/IN sublink is pulled up to a join
Next
From: solai v
Date:
Subject: Re: Add pg_get_publication_ddl function