[PATCH] Fix column name escaping in postgres_fdw stats import

On 4/21/26 4:43 AM, Ayush Tiwari wrote:

Hi hackers,

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.

Reproduction:

  CREATE TABLE t ("col'quote" int, c2 int);
  INSERT INTO t SELECT g, g FROM generate_series(1,100) g;
  ANALYZE t;

  CREATE FOREIGN TABLE ft ("col'quote" int, c2 int)
    SERVER loopback OPTIONS (table_name 't', restore_stats 'true');

  ANALYZE ft;
  -- ERROR:  syntax error at or near "quote"
  -- CONTEXT:  remote SQL command: ... attname = ANY('{...
"col'quote"}'::text[])

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.

Thoughts?

It should also address the issue that was raised in [1].

[1] PostgreSQL: Fix array-element quoting in postgres_fdw import
statistics
</messages/by-id/CAHg+QDc9=WtYi=JW6QUL6ASOJc6PcGPTuxoMkhnkQ7oi7j5atg@mail.gmail.com&gt;

Regards,
Ayush

I think the fix makes sense to me. Here, the column names are emitted as
string content, thus deparseStringLiteral() is a better fit. A small
comment on the test:

+ANALYZE VERBOSE simport_ft_quote;         -- should work, not syntax error

VERBOSE seems not needed.
Regards, Alex Guo

On Tue, Apr 21, 2026 at 3:12 PM Alex Guo <guo.alex.hengchen@gmail.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 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.

Best regards,
Etsuro Fujita

Hi,

Thanks for the review!

On Tue, 21 Apr 2026 at 17:00, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Tue, Apr 21, 2026 at 3:12 PM Alex Guo <guo.alex.hengchen@gmail.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/&gt;

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

On Tue, Apr 21, 2026 at 8:40 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:

On Tue, 21 Apr 2026 at 17:00, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On 4/21/26 4:43 AM, Ayush Tiwari wrote:

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?

No, you shouldn't, but I think it's usual to discuss (essentially) the
same problem in a single place.

Best regards,
Etsuro Fujita

Hi Fujita-san,

On Tue, 21 Apr, 2026, 18:39 Etsuro Fujita, <etsuro.fujita@gmail.com> wrote:

No, you shouldn't, but I think it's usual to discuss (essentially) the
same problem in a single place.

Are there some edits you would like me to make on the patch?

Or would you like me to add the thread link to open items for 19?

Regards,
Ayush

Hi Ayush,

On Mon, May 4, 2026 at 7:24 AM Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:

Are there some edits you would like me to make on the patch?

One thing I noticed is the test case you added; it only tests
departing column names with single quotes, but it's better to also
test that with backslashes, to confirm the patch addresses the issue
in [1]/messages/by-id/CAHg+QDc9=WtYi=JW6QUL6ASOJc6PcGPTuxoMkhnkQ7oi7j5atg@mail.gmail.com as well. So I updated the patch as such. New version
attached. Other than that the patch looks good to me, so I'll push it
if no objections from you (or anyone else).

Or would you like me to add the thread link to open items for 19?

I added this thread to an existing open item as the second report.

Sorry for the delay.

Best regards,
Etsuro Fujita

[1]: /messages/by-id/CAHg+QDc9=WtYi=JW6QUL6ASOJc6PcGPTuxoMkhnkQ7oi7j5atg@mail.gmail.com

Hi Fujita-san,

On Tue, 12 May 2026 at 13:06, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

Hi Ayush,

On Mon, May 4, 2026 at 7:24 AM Ayush Tiwari <ayushtiwari.slg01@gmail.com>
wrote:

Are there some edits you would like me to make on the patch?

One thing I noticed is the test case you added; it only tests
departing column names with single quotes, but it's better to also
test that with backslashes, to confirm the patch addresses the issue
in [1] as well. So I updated the patch as such. New version
attached. Other than that the patch looks good to me, so I'll push it
if no objections from you (or anyone else).

Thanks for the updated version, and for folding in the backslash case
from [1].

I confirmed that the patch applies cleanly on master and that
make -C contrib/postgres_fdw check passes, including the new
dtest_ftable case. The merged test seems a strict improvement.

No objections from me.

Regards,
Ayush

Ayush Tiwari <ayushtiwari.slg01@gmail.com> 于2026年5月12日周二 16:03写道：

Hi Fujita-san,

On Tue, 12 May 2026 at 13:06, Etsuro Fujita <etsuro.fujita@gmail.com>
wrote:

Hi Ayush,

On Mon, May 4, 2026 at 7:24 AM Ayush Tiwari <ayushtiwari.slg01@gmail.com>
wrote:

Are there some edits you would like me to make on the patch?

One thing I noticed is the test case you added; it only tests
departing column names with single quotes, but it's better to also
test that with backslashes, to confirm the patch addresses the issue
in [1] as well. So I updated the patch as such. New version
attached. Other than that the patch looks good to me, so I'll push it
if no objections from you (or anyone else).

Thanks for the updated version, and for folding in the backslash case
from [1].

I confirmed that the patch applies cleanly on master and that
make -C contrib/postgres_fdw check passes, including the new
dtest_ftable case. The merged test seems a strict improvement.

No objections from me.

Regards,
Ayush

The patch overall looks solid to me. A tiny suggestion, while here, would
it make sense to also add comma and brace column names to the test, as they
are classic array-literal hazards, like “col,comma” and “col{brace”?

Regards,
Zhenwei Shang

Hi,

On Wed, May 13, 2026 at 4:27 PM Zhenwei Shang <a934172442@gmail.com> wrote:

Ayush Tiwari <ayushtiwari.slg01@gmail.com> 于2026年5月12日周二 16:03写道：

On Tue, 12 May 2026 at 13:06, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

One thing I noticed is the test case you added; it only tests
departing column names with single quotes, but it's better to also
test that with backslashes, to confirm the patch addresses the issue
in [1] as well. So I updated the patch as such. New version
attached. Other than that the patch looks good to me, so I'll push it
if no objections from you (or anyone else).

Thanks for the updated version, and for folding in the backslash case
from [1].

I confirmed that the patch applies cleanly on master and that
make -C contrib/postgres_fdw check passes, including the new
dtest_ftable case. The merged test seems a strict improvement.

No objections from me.

Cool! Thanks for double-checking, Ayush!

The patch overall looks solid to me. A tiny suggestion, while here, would it make sense to also add comma and brace column names to the test, as they are classic array-literal hazards, like “col,comma” and “col{brace”?

Those cases are handled correctly even without the patch, so sorry, I
don't feel the need to extend the test further.

Anyway, thanks for reviewing, Zhenwei!

Best regards,
Etsuro Fujita

On Wed, May 13, 2026 at 8:22 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

On Wed, May 13, 2026 at 4:27 PM Zhenwei Shang <a934172442@gmail.com> wrote:

The patch overall looks solid to me. A tiny suggestion, while here, would it make sense to also add comma and brace column names to the test, as they are classic array-literal hazards, like “col,comma” and “col{brace”?

Those cases are handled correctly even without the patch, so sorry, I
don't feel the need to extend the test further.

Pushed. I didn't extend the test, but I think we could do so later if needed.

I closed the entry in the PG20-1 CF as well as the open item.

Best regards,
Etsuro Fujita