Horton

Thread

Commits

  1. Fix oversight in commit aa1f93a33.

  2. postgres_fdw: Replace buffers in RemoteAttributeMapping with pointers.

  1. postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import

    Chao Li <li.evan.chao@gmail.com> — 2026-06-11T08:50:28Z

    Hi,

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.

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

  2. Re: postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2026-06-11T10:59:56Z

    Hi Chao,

On Thu, Jun 11, 2026 at 5:51 PM Chao Li <li.evan.chao@gmail.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

  3. Re: postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2026-06-14T07:22:50Z

    On Thu, Jun 11, 2026 at 7:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> 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.

As I couldn't find any, I pushed it.

Best regards,
Etsuro Fujita

  4. Re: postgres_fdw: possible overlooked NAMEDATALEN comparison in stats import

    Chao Li <li.evan.chao@gmail.com> — 2026-06-14T11:31:15Z

    
> On Jun 14, 2026, at 15:22, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> 
> On Thu, Jun 11, 2026 at 7:59 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> 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.
> 
> As I couldn't find any, I pushed it.
> 
> Best regards,
> Etsuro Fujita

Thanks a lot for pushing.

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