(Just relalized this was sent to chap in private, resent it again). On Mon, Aug 21, 2023 at 6:50 PM Andy Fan <zhihui.fan1...@gmail.com> wrote:
> > > On Mon, Aug 21, 2023 at 11:19 AM Chapman Flack <c...@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. > > 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
v10-0003-relabel-the-arg-and-resultvalue-with-INTERNALOID.patch
Description: Binary data
v10-0001-optimize-casting-jsonb-to-a-given-type.patch
Description: Binary data
v10-0002-convert-anyelement-to-internal.patch
Description: Binary data