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

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>
Subject: New "single-call SRF" APIs are very confusingly named
Date: 2022-10-13 19:48:20
Message-ID: 20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I unfortunately just noticed this now, just after we released...

In

commit 9e98583898c347e007958c8a09911be2ea4acfb9
Author: Michael Paquier <michael(at)paquier(dot)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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2022-10-13 19:49:35 Re: Lack of PageSetLSN in heap_xlog_visible
Previous Message Daniel Gustafsson 2022-10-13 19:40:42 Re: remove redundant memset() call