Re: [PATCH] Fix column name escaping in postgres_fdw stats import

From: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Alex Guo <guo(dot)alex(dot)hengchen(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, efujita(at)postgresql(dot)org
Subject: Re: [PATCH] Fix column name escaping in postgres_fdw stats import
Date: 2026-04-21 11:40:07
Message-ID: CAJTYsWXE_nGdVJRXjuNtT4fwbLY98oFKwKRrWwsrpzkc2vjMjw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thanks for the review!

On Tue, 21 Apr 2026 at 17:00, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:

> On Tue, Apr 21, 2026 at 3:12 PM Alex Guo <guo(dot)alex(dot)hengchen(at)gmail(dot)com>
> wrote:
> > On 4/21/26 4:43 AM, Ayush Tiwari wrote:
> > The new statistics import feature in postgres_fdw (commit 28972b6fc3d)
> > builds a remote query to fetch pg_stats rows, filtering by column name
> > with:
> >
> > AND attname = ANY('{col1, col2}'::text[])
> >
> > The column names are formatted with quote_identifier(), which only
> > escapes double quotes. But since the list is embedded inside a
> > single-quoted string literal, any single quote in a column name
> > breaks the literal and produces a syntax error on the remote server.
>
> > The attached patch switches to an ARRAY[] constructor with each
> > element escaped by deparseStringLiteral(), matching how schemaname
> > and tablename are already handled in the same function.
>
> Thanks for the report and patch!
>
> > It should also address the issue that was raised in [1].
>
> The root cause of this is the same as [1], so I think you should reply
> to the thread, rather than creating a new thread.
>

I faced the issue with the quoting scenario and was unaware of [1] then,
and that patch did not solve the issue regarding the quotes, which is
why I started this. Should I move this there? I've registered it in
commitfest: Fix column name escaping in postgres_fdw stats import
<https://commitfest.postgresql.org/patch/6695/>

>
> > I think the fix makes sense to me. Here, the column names are emitted as
> string content, thus deparseStringLiteral() is a better fit.
>
> +1
>
> > A small comment on the test:
> >
> > +ANALYZE VERBOSE simport_ft_quote; -- should work, not syntax
> error
> >
> > VERBOSE seems not needed.
>
> I think the option is needed; otherwise we cannot check that stats
> import was really done in the test.
>

Yeah, that was the intention.

Regards,
Ayush

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message lakshmi 2026-04-21 11:58:19 Re: ECPG: inconsistent behavior with the document in “GET/SET DESCRIPTOR.”
Previous Message cca5507 2026-04-21 11:35:58 Re: Return value of XLogInsertRecord() for XLOG_SWITCH record