Re: New "single-call SRF" APIs are very confusingly named

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>
Subject: Re: New "single-call SRF" APIs are very confusingly named
Date: 2022-10-14 01:28:34
Message-ID: Y0i7QkAwXH8Ez9e/@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 13, 2022 at 12:48:20PM -0700, Andres Freund wrote:
> I unfortunately just noticed this now, just after we released...

Thanks for the feedback.

> Even leaving the confusion with ExprSingleResult aside, calling it "Single"
> still seems very non-descriptive. I assume it's named to contrast with
> init_MultiFuncCall() etc. While those are also not named greatly, they're not
> typically used directly but wraped in SRF_FIRSTCALL_INIT etc.

This is mentioned on the original thread here, as of the point that we
go through the function one single time:
https://www.postgresql.org/message-id/Yh8SBTuzKhq7Jwda@paquier.xyz

> I also quite intensely dislike SRF_SINGLE_USE_EXPECTED. It sounds like it's
> saying that a single use of the SRF is expected, but that's not at all what it
> means: "use expectedDesc as tupdesc".

Okay. Something like the USE_EXPECTED_DESC you are suggesting or
USE_EXPECTED_TUPLE_DESC would be fine by me.

> I'm also confused by SRF_SINGLE_BLESS - the comment says "validate tuple for
> SRF". BlessTupleDesc can't really be described as validation, or am I missing
> something?

I'd rather keep the flag name to match the history behind this API.
How about updating the comment as of "complete tuple descriptor, for a
transient RECORD datatype", or something like that?

> Maybe something like InitMaterializedSRF() w/
> MAT_SRF_(USE_EXPECTED_DESC|BLESS)

Or just SetMaterializedFuncCall()? Do we always have to mention the
SRF part of it once we tell about the materialization part? The
latter sort implies the former once a function returns multiple
tuples.

I don't mind doing some renaming of all that even post-release, though
comes the question of keeping some compabitility macros for
compilation in case one uses these routines? Any existing extension
code works out-of-the-box without these new routines, so the chance of
somebody using the new stuff outside core sounds rather limited but it
does not seem worth taking a risk, either.. And this has been in the
tree for a bit more than half a year now.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-10-14 01:34:26 Re: New "single-call SRF" APIs are very confusingly named
Previous Message Peter Geoghegan 2022-10-14 01:24:27 Re: hash_xlog_split_allocate_page: failed to acquire cleanup lock