Re: Extract numeric filed in JSONB more effectively

On Mon, Aug 21, 2023 at 11:19 AM Chapman Flack <chap@anastigmatix.net> wrote:

On 2023-08-20 21:31, Andy Fan wrote:
> Highlighting the user case of makeRelableType is interesting! But using
> the Oid directly looks more promising for this question IMO, it looks
> like:
> "you said we can put anything in this arg,  so I put an OID const
> here",
> seems nothing is wrong.

Perhaps one of the more senior developers will chime in, but to me,
leaving out the relabel nodes looks more like "all of PostgreSQL's
type checking happened before the SupportRequestSimplify, so nothing
has noticed that we rewrote the tree with mismatched types, and as
long as nothing crashes we sort of got away with it."

Suppose somebody writes an extension to double-check that plan
trees are correctly typed. Or improves EXPLAIN to check a little more
carefully than it seems to. Omitting the relabel nodes could spell
trouble then.

Or, someone more familiar with the code than I am might say "oh,
mismatches like that are common in rewritten trees, we live with it."
But unless somebody tells me that, I'm not believing it.

Well, this sounds long-lived.  I kind of prefer to label it now.  Adding

the 3rd commit to relabel the arg and return value. 

 

But I would say we have proved the concept of SupportRequestSimplify
for this task. :)

Yes,  this is great! 

 

Now, it would make me happy to further reduce some of the code
duplication between the original and the _type versions of these
functions. I see that you noticed the duplication in the case of
jsonb_extract_path, and you factored out jsonb_get_jsonbvalue so
it could be reused. There is also some duplication with object_field
and array_element.

(Also, we may have overlooked jsonb_path_query
and jsonb_path_query_first as candidates for the source of the
cast. Two more candidates; five total.)

Here is one way this could be structured. Observe that every one
of those five source candidates operates in two stages:

I'm not very excited with this manner, reasons are: a).  It will have

to emit more steps in ExprState->steps which will be harmful for

execution. The overhead  is something I'm not willing to afford.

b). this manner requires more *internal*, which is kind of similar

to "void *"  in C.  Could you explain more about the benefits of this? 

--

Best Regards

Andy Fan