Re: Patch: Code comments: why some text-handling functions are leakproof

I'm going to mark this returned with feedback.

If you have a chance to update the patch moving the documentation to
xfunc.sgml the way Tom describes make sure to create a new commitfest
entry. I would suggest submitting the patch as a followup on this
thread so when it's added to the commitfest it links to this whole
discussion.


On Mon, 28 Feb 2022 at 17:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Tue, Jan 11, 2022 at 2:07 AM Gurjeet Singh <gurjeet@singh.im> wrote:
> >> This is more or less a verbatim copy of Tom's comment in email thread at [1].
> >>
> >> I could not find an appropriate spot to place these comments, so I placed them on bttextcmp() function, The only
otherplace that I could see we can place these comments is in the file src/backend/optimizer/README, because there is
someconsideration given to leakproof functions in optimizer docs. But these comments seem quite out of place in
optimizerdocs. 
>
> > It doesn't seem particularly likely that someone who is thinking about
> > changing this in the future would notice the comment in the place
> > where you propose to put it, nor that they would read the optimizer
> > README.
>
> Agreed.  I think if we wanted to make an upgrade in the way function
> leakproofness is documented, we ought to add a <sect1> about it in
> xfunc.sgml, adjacent to the one about function volatility categories.
> This could perhaps consolidate some of the existing documentation mentions
> of leakproofness, as well as adding text similar to what Gurjeet suggests.
>
> > Furthermore, I don't know that everyone agrees with Tom about this. I
> > do agree that it's more important to mark relational operators
> > leakproof than other things, and I also agree that conservatism is
> > warranted. But that does not mean that someone could not make a
> > compelling argument for marking other functions leakproof.
>
> ISTM the proposed text does a reasonable job of explaining why
> we made the decisions currently embedded in pg_proc.proleakproof.
> If we make some other decisions in future, updating the rationale
> in the docs would be an appropriate part of that.
>
>                         regards, tom lane
>
>


--
greg