Re: Extract numeric filed in JSONB more effectively - Mailing list pgsql-hackers
From | Chapman Flack |
---|---|
Subject | Re: Extract numeric filed in JSONB more effectively |
Date | |
Msg-id | b2b630ff9804f9eae8205ad721fd0682@anastigmatix.net Whole thread Raw |
In response to | Re: Extract numeric filed in JSONB more effectively (Andy Fan <zhihui.fan1213@gmail.com>) |
Responses |
Re: Extract numeric filed in JSONB more effectively
Re: Extract numeric filed in JSONB more effectively |
List | pgsql-hackers |
On 2023-08-17 21:14, Andy Fan wrote: >> The idea of an 'internal' return type with no 'internal' parameter >> was quickly and rightly shot down. > > Yes, it mainly breaks the type-safety system. Parser need to know > the result type, so PG defines the rule like this: Well, the reason "internal return type with no internal parameter type" was shot down was more specific: if there is such a function in the catalog, an SQL user can call it, and then its return type is a value typed 'internal', and with that the SQL user could call other functions with 'internal' parameters, and that's what breaks type safety. The specific problem is not having at least one 'internal' input parameter. There are lots of functions in the catalog with internal return type (I count 111). They are not inherently bad; the rule is simply that each one also needs at least one IN parameter typed internal, to make sure it can't be directly called from SQL. > anyelement fn(anyment in); > > if the exprType(in) in the query tree is X, then PG would think fn > return type X. that's why we have to have an anyelement in the > input. That's a consequence of the choice to have anyelement as the return type, though. A different choice wouldn't have that consequence. > I have some trouble understanding this. are you saying something > like: > > internal fn(internal jsonValue, internal typeOid)? > > If so, would it break the type-safety system? That is what I'm saying, and it doesn't break type safety at the SQL level, because as long as it has parameters declared internal, no SQL can ever call it. So it can only appear in an expression tree because your SupportRequestSimplify put it there properly typed, after the SQL query was parsed but before evaluation. The thing about 'internal' is it doesn't represent any specific type, it doesn't necessarily represent the same type every time it is mentioned, and it often means something that isn't a cataloged type at all, such as a pointer to some kind of struct. There must be documentation explaining what it has to be. For example, your jsonb_cast_support function has an 'internal' parameter and 'internal' return type. From the specification for support functions, you know the 'internal' for the parameter type means "one of the Node structs in supportnodes.h", and the 'internal' for the return type means "an expression tree semantically equivalent to the FuncExpr". So, in addition to declaring internal fn(internal jsonValue, internal typeOid), you would have to write a clear spec that jsonValue has to be a JsonbValue, typeOid has to be something you can call DatumGetObjectId on, and the return value should be a Datum in proper form corresponding to typeOid. And, of course, generate the expression tree so all of that is true when it's evaluated. >> Perhaps there are parts of that rewriting that no existing node type >> can represent? The description above was in broad strokes. Because I haven't tried to implement this, I don't know whether some roadblock would appear, such as, is it hard to make a Const node of type internal and containing an oid? Or, what sort of node must be inserted to clarify that the 'internal' return is actually a Datum of the expected type? By construction, we know that it is, but how to make that explicit in the expression tree? > a). we can't use the makeNullConst because jsonb_xxx_type is > strict, so if we have NULL constant input here, the PG system > will return NULL directly. b). Not only the type oid is the thing > We are interested in the const.constvalue is as well since > 'explain select xxxx' to access it to show it as a string. > Datum(0) as the constvalue will crash in this sense. That's why > makeDummyConst was introduced. Again, all of that complication stems from the choice to use the anyelement return type and rely on polymorphic type resolution to figure the oid out, when we already have the oid to begin with and the oid is all we want. >> something like "assertion of >> 'internal'-to-foo binary coercibility, vouched by a prosupport >> function", would that be a bad thing? > > I can't follow this as well. That was just another way of saying what I was getting at above about what's needed in the expression tree to indicate that the 'internal' produced by this function is, in fact, really a bool (or whatever). We know that it is, but perhaps the expression tree will be considered ill-formed without a node that says so. A node representing a no-op, binary conversion would suffice, but is there already a node that's allowed to represent an internal-to-bool no-op cast? If there isn't, one might have to be invented. So it might be that if we go down the "use polymorphic resolution" road, we have to invent dummy Consts, and down the "internal" road we also have to invent something, like the "no-op cast considered correct because a SupportRequestSimplify function put it here" node. If it came down to having to invent one of those things or the other, I'd think the latter more directly captures what we really want to do. (And I'm not even sure anything has to be invented. If there's an existing node for no-op binary casts, I think I'd first try putting that there and see if anything complains.) If this thread is being followed by others more familiar with the relevant code or who see obvious problems I'm missing, please chime in! Regards, -Chap
pgsql-hackers by date: