Re: postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import

Hi Chao,

On Thu, Jun 11, 2026 at 5:51 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> I’m testing "[28972b6fc] Add support for importing statistics from remote servers". Functionally, so far so good. I just found 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-byte array to a char pointer, and its commit message explicitly says "the remote column name in particular could be longer than NAMEDATALEN - 1". In that case, remattrmap_cmp() could treat two distinct remote column names as equal for 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. I made a local change as the attached diff, and no test is broken with the change.

You are right; that's an oversight in the follow-up commit. The patch
looks good to me, so I'll push it after confirming that there are no
similar mistakes.

Thanks for the report and patch!

Best regards,
Etsuro Fujita