Re: Converting contrib SQL functions to new style

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