On Wed, Nov 13, 2024 at 09:15:08AM +0100, Ronan Dunklau wrote: > Ok, please find attached a new complete patch series including tests for the > uncovered functions. Tests pass both before and after the move to SQL-body > functions.
In 0001 for the tests of contrib/xml2/, you have forgotten to update the alternate output used when compiling with xml2 and without libxslt. Fixed that, then applied. 0002 was OK as-is, so applied. 0006 to switch contrib/lo/ is so short that using named arguments does not change much IMO, so I've left it as you proposed, then applied it. pg_freespace with two functions is small enough that it does not seem worth bothering more, as well. Remains the cases of citext, pageinspect, xml2 and earthdistance. The argument of using named parameters is appealing to self-document the functions, but it seems to me that it gets much better once we also do the same for all the functions of a given extension, not only these whose body is changed. This information does not show up in a \dx+, it does in a \df. Doing that step-by-step is better than nothing, hence limiting the use of named parameters for only the functions whose body is rewritten is fine by me, as a first step, as long as the names are used rather the dollar parameter numbers. I'd suggest to do take the bonus step of applying the same rule to all the other functions so as everything applies with the same extension update in a single major release. Perhaps on top of the patches already proposed? There is no need for an extra version bump if all that is done in the same development cycle. Thoughts? -- Michael
signature.asc
Description: PGP signature
