synchronous_standby_names parser discards errors

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: synchronous_standby_names parser discards errors
Date: 2025-07-24 17:58:14
Message-ID: CA+TgmoYF9wPNZ-Q_EMfib_espgHycY-eX__6Tzo2GpYpVXqCdQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Since Peter Eisentraut's commit
473a575e05979b4dbb28b3f2544f4ec8f184ce65 on January 25 of this year,
attempting to set synchronous_standby_names to a value that doesn't
parse results in a generic and uninformative error message:

[robert.haas ~]$ postgres -c synchronous_standby_names='"foo'
2025-07-24 17:16:05.769 GMT [35558] FATAL: invalid value for
parameter "synchronous_standby_names": ""foo"
2025-07-24 17:16:05.769 GMT [35558] DETAIL:
"synchronous_standby_names" parser failed.

This happens because syncrep_yyerror() claims to stuff the error
message into *syncrep_parse_error_msg_p but does not actually do so.
The attached patch fixes it:

[robert.haas ~]$ postgres -c synchronous_standby_names='"foo'
2025-07-24 17:16:22.527 GMT [35962] FATAL: invalid value for
parameter "synchronous_standby_names": ""foo"
2025-07-24 17:16:22.527 GMT [35962] DETAIL: unterminated quoted
identifier at end of input

Apparently, our tests do not exercise this parser's error cases. I
looked into adding a test but I don't see a real easy place to add
one. If we had testing for messages like "WAL archival cannot be
enabled when \"wal_level\" is \"minimal\"" then it would make sense to
test this kind of thing in the same place, but we don't. Because the
setting is PGC_SIGHUP, we can't just include a SET command in the main
regression tests, either. I don't want to spend the time to upgrade
our testing infrastructure to catch future instances of this problem
right now, and therefore propose to just commit this fix and move on.

Thanks to my colleague Jacob Champion for helping track this down.

--
Robert Haas
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v1-0001-Avoid-throwing-away-the-error-message-in-syncrep_.patch application/octet-stream 1.7 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2025-07-24 18:02:39 Re: Non-text mode for pg_dumpall
Previous Message Noah Misch 2025-07-24 17:50:36 Re: Non-text mode for pg_dumpall