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-23T19:33:51Z
Lists: pgsql-hackers
Attachments
- rework-tsearch-readline-3.patch (text/x-diff) patch
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
Commits
-
Improve behavior of tsearch_readline(), and remove t_readline().
- 83b61319a16c 14.0 landed
-
Avoid possible dangling-pointer access in tsearch_readline_callback.
- c1f63c42fc13 11.10 landed
- b91033ae9bc5 10.15 landed
- b1fbb7d08f22 9.6.20 landed
- 61cfd90621d6 9.5.24 landed
- 5d0c80658328 12.5 landed
- 569f6a89a915 13.1 landed
- 3ea7e9550e58 14.0 landed
-
Simplify SortTocFromFile() by removing fixed buffer-size limit.
- 2e3c19462da9 14.0 landed
-
Remove arbitrary line length limit for libpq service files.
- c0cb87fbb664 14.0 landed
-
Rethink API for pg_get_line.c, one more time.
- 931487018c40 14.0 landed