Re: Lift line-length limit for pg_service.conf

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Lift line-length limit for pg_service.conf
Date: 2020-09-23 19:33:51
Message-ID: 1504903.1600889631@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> So the attached adds a pstrdup/pfree to ensure that "curline"
> has its own storage, putting us right back at two palloc/pfree
> cycles per line. I don't think there's a lot of choice though;
> in fact, I'm leaning to the idea that we need to back-patch
> that part of this. The odds of trouble in a production build
> probably aren't high, but still...

So I did that, but while looking at the main patch I realized that
a few things were still being left on the table:

1. We can save a palloc/pfree cycle in the case where no encoding
conversion need be performed, by allowing "curline" to point at
the StringInfo buffer instead of necessarily being a separate
palloc'd string. (This seems safe since no other code should
manipulate the StringInfo before the next tsearch_readline call;
so we can't get confused about whether "curline" has its own storage.)

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.

3. AFAICT, it's completely useless for tsearch_readline to call
pg_verify_mbstr, because pg_any_to_server will either do that exact
thing itself, or verify the input encoding as part of conversion.
Some quick testing says that you don't even get different error
messages. So we should just drop that step. (It's likely that the
separate call was actually needed when this code was written; I think
we tightened up the expectations for verification within encoding
conversion somewhere along the line. But we demonstrably don't need
it now.)

So those considerations lead me to the attached.

regards, tom lane

Attachment Content-Type Size
rework-tsearch-readline-3.patch text/x-diff 5.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabrízio de Royes Mello 2020-09-23 21:11:06 Re: Missing TOAST table for pg_class
Previous Message Andres Freund 2020-09-23 19:33:17 Re: Batching page logging during B-tree build