Re: [HACKERS] [PATCH] Generic type subscripting

I decided that the way to get this moved forward is to ignore the jsonb
parts for the moment and focus on getting the core feature into
committable shape.  It's possible that the lack of a concrete use-case
other than arrays will cause us to miss a detail or two, but if so we
can fix it later, I think.  (We should make sure to get the jsonb parts
in for v14, though, before we ship this.)

Accordingly, I went through all of the core and array code and dealt
with a lot of details that hadn't gotten seen to, including pg_dump
support and dependency considerations.  I ended up rewriting the
parser code pretty heavily, because I didn't like the original either
from an invasiveness or usability standpoint.  I also did the thing
I suggested earlier of using separate handler functions for varlena
and fixed-length arrays, though I made no effort to separate the code
paths.  I think the attached v37 is committable or nearly so, though
there remain a few loose ends:

1. I'm still wondering if TypeParamBool is the right thing to pass to
LLVMFunctionType() to describe a function-returning-bool.  It does
seem to work on x64_64 and aarch64, for what that's worth.

2. I haven't pulled the trigger on merging the three now-identical
execution step types.  That could be done separately, of course.

3. There are some semantic details that probably need debating.
As I wrote in subscripting.h:

 * There are some general restrictions on what subscripting can do.  The
 * planner expects subscripting fetches to be strict (i.e., return NULL for
 * any null input), immutable (same inputs always give same results), and
 * leakproof (data-value-dependent errors must not be thrown; in other
 * words, you must silently return NULL for any bad subscript value).
 * Subscripting assignment need not be, and usually isn't, strict; it need
 * not be leakproof either; but it must be immutable.

I doubt that there's anything too wrong with assuming immutability
of SubscriptingRef.  And perhaps strictness is OK too, though I worry
about somebody deciding that it'd be cute to define a NULL subscript
value as doing something special.  But the leakproofness assumption
seems like a really dangerous one.  All it takes is somebody deciding
that they should throw an error for a bad subscript, and presto, we
have a security hole.

What I'm slightly inclined to do here, but did not do in the attached
patch, is to have check_functions_in_node look at the leakproofness
marking of the subscripting support function; that's a bit of a hack,
but since the support function can't be called from SQL there is no
other use for its proleakproof flag.  Alternatively we could extend
the SubscriptingRoutine return struct to include some flags saying
whether fetch and/or assignment is leakproof.

BTW, right now check_functions_in_node() effectively treats
SubscriptingRef as unconditionally leakproof.  That's okay for the
array "fetch" code, but the array "store" code does throw errors
for bad subscripts, meaning it's not leakproof.  The only reason
this isn't a live security bug is that "store" SubscriptingRefs can
only occur in the targetlist of an INSERT or UPDATE, which is not
a place that could access any variables we are trying to protect
with the leakproofness mechanism.  (If it could, you could just
store the variable's value directly into another table.)  Still,
that's a shaky chain of reasoning.  The patch's change to
check_functions_in_node() would work in the back branches, so
I'm inclined to back-patch it that way even if we end up doing
something smarter in HEAD.

Comments?

			regards, tom lane