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