Re: "pgoutput" options missing on documentation

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: emre(at)hasegeli(dot)com
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: "pgoutput" options missing on documentation
Date: 2023-12-19 06:36:55
Message-ID: CAHut+Ps1MqsyFmmx=ytFVzrdo-un6vp8rGC7Q4xhWehE=u3-KQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 19, 2023 at 1:35 AM Emre Hasegeli <emre(at)hasegeli(dot)com> wrote:
>
> > Fair enough. I think we should push your first patch only in HEAD as
> > this is a minor improvement over the current behaviour. What do you
> > think?
>
> I agree.

Patch 0001

AFAICT parse_output_parameters possible errors are never tested. For
example, there is no code coverage [1] touching any of these ereports.

IMO there should be some simple test cases -- I am happy to create
some tests if you agree they should exist.

~~~

While looking at the function parse_output_parameters() I noticed that
if an unrecognised option is passed the function emits an elog instead
of an ereport

------
test_pub=# SELECT * FROM pg_logical_slot_get_changes('test_slot_v1',
NULL, NULL, 'banana', '1');
2023-12-19 17:08:21.627 AEDT [8921] ERROR: unrecognized pgoutput option: banana
2023-12-19 17:08:21.627 AEDT [8921] CONTEXT: slot "test_slot_v1",
output plugin "pgoutput", in the startup callback
2023-12-19 17:08:21.627 AEDT [8921] STATEMENT: SELECT * FROM
pg_logical_slot_get_changes('test_slot_v1', NULL, NULL, 'banana',
'1');
ERROR: unrecognized pgoutput option: banana
CONTEXT: slot "test_slot_v1", output plugin "pgoutput", in the startup callback
------

But that doesn't seem right. AFAIK elog messages use errmsg_internal
so this message would not get translated.

PSA a patch to fix that.

======
[1] code coverage --
https://coverage.postgresql.org/src/backend/replication/pgoutput/pgoutput.c.gcov.html

Kind Regards,
Peter Smith.
Fujitsu Australia

Attachment Content-Type Size
parse_output_parameters.diff application/octet-stream 626 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Japin Li 2023-12-19 06:58:30 Re: [DOC] Introducing Quick Start Guide to PL/pgSQL and PL/Python Documentation
Previous Message Richard Guo 2023-12-19 06:23:25 Update the comment in nodes.h to cover Cardinality