Hi,
I unfortunately just noticed this now, just after we released...
In
commit 9e98583898c347e007958c8a09911be2ea4acfb9
Author: Michael Paquier <michael@paquier.xyz>
Date: 2022-03-07 10:26:29 +0900
Create routine able to set single-call SRFs for Materialize mode
a new helper was added:
#define SRF_SINGLE_USE_EXPECTED 0x01 /* use expectedDesc as tupdesc */
#define SRF_SINGLE_BLESS 0x02 /* validate tuple for SRF */
extern void SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags);
I think the naming here is very poor. For one, "Single" here conflicts with
ExprSingleResult which indicates "expression does not return a set",
i.e. precisely the opposite what SetSingleFuncCall() is used for. For another
the "Set" in SetSingleFuncCall makes it sound like it's function setting a
property.
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.
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".
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?
This IMO needs to be cleaned up.
Maybe something like InitMaterializedSRF() w/
MAT_SRF_(USE_EXPECTED_DESC|BLESS)
Greetings,
Andres Freund
