Re: Extract numeric filed in JSONB more effectively

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: Chapman Flack <chap(at)anastigmatix(dot)net>
Cc: jian he <jian(dot)universality(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extract numeric filed in JSONB more effectively
Date: 2023-08-22 03:14:48
Message-ID: CAKU4AWrs4Pzajm2_tgtUTf=CWfDJEx=3h45Lhqg7tNOVZw5YxA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(Just relalized this was sent to chap in private, resent it again).

On Mon, Aug 21, 2023 at 6:50 PM Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com> wrote:

>
>
> On Mon, Aug 21, 2023 at 11:19 AM Chapman Flack <chap(at)anastigmatix(dot)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.
>
>
Yes, compared with jsonb_extract_path, object_field and array_element
just have much less duplication, which are 2 lines and 6 lines separately.

> (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.)
>>
>
I can try to add them at the same time when we talk about the
infrastruct, thanks for the hint!

>> 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
>

--
Best Regards
Andy Fan

Attachment Content-Type Size
v10-0003-relabel-the-arg-and-resultvalue-with-INTERNALOID.patch application/octet-stream 6.1 KB
v10-0001-optimize-casting-jsonb-to-a-given-type.patch application/octet-stream 31.7 KB
v10-0002-convert-anyelement-to-internal.patch application/octet-stream 11.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2023-08-22 03:17:45 Re: [PATCH] Add function to_oct
Previous Message Richard Guo 2023-08-22 03:12:07 Re: Oversight in reparameterize_path_by_child leading to executor crash