Re: Lift line-length limit for pg_service.conf

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-09-23T23:37:54Z
Lists: pgsql-hackers
Daniel Gustafsson <daniel@yesql.se> writes:
> On 23 Sep 2020, at 21:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 2. In the case where encoding conversion is performed, we still have
>> to pstrdup the result to have a safe copy for "curline".  But I
>> realized that we're making a poor choice of which copy to return to
>> the caller.  The output of encoding conversion is likely to be a good
>> bit bigger than necessary, cf. pg_do_encoding_conversion.  So if the
>> caller is one that saves the output string directly into a long-lived
>> dictionary structure, this wastes space.  We should return the pstrdup
>> result instead, and keep the conversion result as "curline", where
>> we'll free it next time through.

> I wonder if we have other callsites of pg_any_to_server which could benefit
> from lowering the returned allocation, a quick look didn't spot any but today
> has become yesterday here and tiredness might interfere.

After looking more closely, I've realized that actually none of the
existing core-code callers will save the returned string directly.
readstoplist() could do so, depending on what "wordop" is, but
all its existing callers pass lowerstr() which will always make a
new output string.  (Which itself could be a bit bloated :-()

So the concern I expressed above is really just hypothetical.
Still, the code is simpler this way and no slower, so I still
think it's an improvement.

(The bigger picture here is that the API for dictionary init
methods is pretty seriously misdesigned from a memory-consumption
standpoint.  Running the entire init process in the dictionary's
long-lived context goes against everything we've learned about
avoiding memory leaks.  We should run that code in a short-lived
command execution context, and explicitly copy just the data we
want into the long-lived context.  But changing that would be
a pretty big deal, breaking third-party dictionaries.  So I'm
not sure it's enough of a problem to justify the change.)

			regards, tom lane



Commits

  1. Improve behavior of tsearch_readline(), and remove t_readline().

  2. Avoid possible dangling-pointer access in tsearch_readline_callback.

  3. Simplify SortTocFromFile() by removing fixed buffer-size limit.

  4. Remove arbitrary line length limit for libpq service files.

  5. Rethink API for pg_get_line.c, one more time.