Re: pipe_read_line for reading arbitrary strings

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pipe_read_line for reading arbitrary strings
Date: 2023-11-24 10:08:54
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 22 Nov 2023, at 13:47, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> On 2023-Mar-07, Daniel Gustafsson wrote:
>> The attached POC diff replace fgets() with pg_get_line(), which may not be an
>> Ok way to cross the streams (it's clearly not a great fit), but as a POC it
>> provided a neater interface for reading one-off lines from a pipe IMO. Does
>> anyone else think this is worth fixing before too many callsites use it, or is
>> this another case of my fear of silent subtle truncation bugs? =)
> I think this is generally a good change.

Thanks for review!

> I think pipe_read_line should have a "%m" in the "no data returned"
> error message.

Good point.

> pg_read_line is careful to retain errno (and it was
> already zero at start), so this should be okay ... or should we set
> errno again to zero after popen(), even if it works?

While it shouldn't be needed, reading manpages from a variety of systems
indicates that popen() isn't entirely reliable when it comes to errno so I've
added an explicit errno=0 just to be certain.

> (I'm not sure I buy pg_read_line's use of perror in the backend case.
> Maybe this is only okay because the backend doesn't use this code?)

In EXEC_BACKEND builds the postmaster will use find_other_exec which in turn
calls pipe_read_line, so there is a possibility. I agree it's proabably not a
good idea, I'll have a look at it separately and will raise on a new thread.

> pg_get_line says caller can distinguish error from no-input-before-EOF
> with ferror(), but pipe_read_line does no such thing. I wonder what
> happens if an NFS mount containing a file being read disappears in the
> middle of reading it, for example ... will we think that we completed
> reading the file, rather than erroring out?

Interesting, that's an omission which should be fixed. I notice there are a
number of callsites using pg_get_line which skip validating with ferror(), I'll
have a look at those next (posting findings to a new thread).

Daniel Gustafsson

Attachment Content-Type Size
v4-0001-Refactor-pipe_read_line-to-return-the-full-line.patch application/octet-stream 6.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Alena Rybakina 2023-11-24 10:20:52 Re: Reducing memory consumed by RestrictInfo list translations in partitionwise join planning
Previous Message Alvaro Herrera 2023-11-24 09:53:40 Re: GUC names in messages