stray references to SubscriptRef type
There are several mentions in code comments of a SubscriptRef type, but
that type does not exist and AFAICT never existed. (Perhaps it existed
in in-development code at some point.)
I think in several of those cases this clearly meant the type
SubscriptingRef instead. But the mentions in
src/include/nodes/subscripting.h probably meant the various sbs_* functions.
If so, it would also be useful perhaps to document the leakproof
expectations of the other sbs_* functions (sbs_fetch_old and
sbs_check_subscripts).
See attached patch for my solution so far.
Attachments:
0001-Fix-stray-references-to-SubscriptRef.patchtext/plain; charset=UTF-8; name=0001-Fix-stray-references-to-SubscriptRef.patchDownload+10-11
Peter Eisentraut <peter@eisentraut.org> writes:
There are several mentions in code comments of a SubscriptRef type, but
that type does not exist and AFAICT never existed. (Perhaps it existed
in in-development code at some point.)
Yeah, I think we went back and forth on the node name for awhile
before settling on SubscriptingRef. These comments evidently escaped
being updated.
I think in several of those cases this clearly meant the type
SubscriptingRef instead. But the mentions in
src/include/nodes/subscripting.h probably meant the various sbs_* functions.
I'm fairly certain that the SubscriptingRef node type was meant,
but I tend to agree that refocusing those comments on the method
functions would be an improvement. However, the way you've written it
here seems incorrect. What the comment is trying to say is that the
entire operation represented by the SubscriptingRef node is strict or
leakproof if those flags are set. Thus for example, fetch_leakproof
asserts that both sbs_fetch and sbs_check_subscripts (if used) are
leakproof. store_leakproof implicates sbs_check_subscripts,
sbs_assign, and sbs_fetch_old.
If so, it would also be useful perhaps to document the leakproof
expectations of the other sbs_* functions (sbs_fetch_old and
sbs_check_subscripts).
See above.
regards, tom lane