MIME-Version: 1.0
Date: Thu, 17 Aug 2023 16:30:41 -0400
From: Chapman Flack <chap@anastigmatix.net>
To: Andy Fan <zhihui.fan1213@gmail.com>
Cc: jian he <jian.universality@gmail.com>, Pavel Stehule
 <pavel.stehule@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers
 <pgsql-hackers@postgresql.org>
Subject: Re: Extract numeric filed in JSONB more effectively
In-Reply-To: <CAKU4AWqbd_oDwXyK7=yMKbhAR=CQtVOWszcgft+cTG6JCTKmzQ@mail.gmail.com>
References: <CAKU4AWoqAVya6PBhn+BCbFaBMt3z-2=i5fKO3bW=6HPhbid2Dw@mail.gmail.com>
 <CACJufxHASOEpngQ8V2tbXgs4VZC3ETrVbS=uk0KC_B_J1j7ejQ@mail.gmail.com>
 <CAKU4AWrap1zpYqunJwWTN=CdP7E8e0U4mYmwn7hvTW3ERuENVg@mail.gmail.com>
 <CAFj8pRD-R-GsGCjeYApbhZoiW8TV6zACaYStMBMM0=--+WgN_A@mail.gmail.com>
 <CAKU4AWpDdFXAD+dMC1HeErXSKBUUBRGWkf=dAcX3wZgBNsWM=g@mail.gmail.com>
 <CAFj8pRAO3oEiBaJJ9=HZp6CoP2ffbwSgrKkLKjPfYZwx9wOOuQ@mail.gmail.com>
 <CAKU4AWoCHpKAVuQeOrk44cVPy_dVxn1aHrMUvHy5Ag-daFCSsQ@mail.gmail.com>
 <CAFj8pRD4cdUmK0RG4oN5B2KRSeDhwfMYaL=XpfEu4iaLeZ_Kow@mail.gmail.com>
 <CAKU4AWp8ab61e96v57OaB-Gm1bMfBNVLVy+s17U6_Ne3veB84g@mail.gmail.com>
 <3507485.1691090027@sss.pgh.pa.us>
 <CAKU4AWp+KLes8g=BWLqZfDmW9+=ZY0UC4G0i3qVcYEviK_dDTA@mail.gmail.com>
 <900892914fdc3f477b101d699efb40e0@anastigmatix.net>
 <903341.1692022214@sss.pgh.pa.us>
 <CAKU4AWrBY9GHj9oZbvhiOG1BgiWyZC8FGPAET-CfRKDhYyv1HQ@mail.gmail.com>
 <CAKU4AWrGM5bK7wi4Y8bTYhKgh=A1fW=X00eC_jfk6_JXyaEURQ@mail.gmail.com>
 <CAFj8pRC+4pvSuibB2xcNKJ=6PSF=TAcOtRNpdLPBXZjijFg7ag@mail.gmail.com>
 <CAKU4AWrxHFVZM-gGPpOrVPreZMePAOoY580Tq-+CvxDWHmP_uA@mail.gmail.com>
 <CAKU4AWp3410VpYFdCxpFdaHc8he6zj_=Fvww53TnU+g-bvvvsQ@mail.gmail.com>
 <CACJufxH7ftu9HD+h_gDWPDvq1ZO8vGm81JomSKjvQacCLMLcxg@mail.gmail.com>
 <CAKU4AWqbd_oDwXyK7=yMKbhAR=CQtVOWszcgft+cTG6JCTKmzQ@mail.gmail.com>
User-Agent: Roundcube Webmail/1.6.0
Message-ID: <4b97f1a1dd9b6e45443d24870d3be698@anastigmatix.net>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/4b97f1a1dd9b6e45443d24870d3be698%40anastigmatix.net>
Precedence: bulk

On 2023-08-17 05:07, Andy Fan wrote:
> Thanks for the review,  v9 attached!

 From the earliest iterations of this patch, I seem to recall
a couple of designs being considered:

In one, the type-specific cast function would only be internally
usable, would take a type oid as an extra parameter (supplied in
the SupportRequestSimplify rewriting), and would have to be
declared with some nonspecific return type; 'internal' was
mentioned.

The idea of an 'internal' return type with no 'internal' parameter
was quickly and rightly shot down. But it would have seemed to me
enough to address that objection by using 'internal' also in its
parameter list. I could imagine a function declared with two
'internal' parameters, one understood to be a JsonbValue and one
understood to be a type oid, and an 'internal' result, treated in
the rewritten expression tree as binary-coercible to the desired
result.

Admittedly, I have not tried to implement that myself to see
what unexpected roadblocks might exist on that path. Perhaps
there are parts of that rewriting that no existing node type
can represent? Someone more familiar with those corners of
PostgreSQL may immediately see other difficulties I do not.

But I have the sense that that approach was abandoned early, in
favor of the current approach using user-visible polymorphic
types, and supplying typed dummy constants for use in the
resolution of those types, with a new function introduced to create
said dummy constants, including allocation and input conversion
in the case of numeric, just so said dummy constants can be
passed into functions that have no use for them other than to
call get_fn_expr_argtype to recover the type oid, which was the
only thing needed in the first place.

Compared to the initial direction I thought this was going,
none of that strikes me as better.

Nothing makes my opinion authoritative here, and there may
indeed be reasons it is better, known to others more familiar
with that code than I am. But it bugs me.

If the obstacles to the earlier approach came down to needing
a new type of expression node, something like "assertion of
'internal'-to-foo binary coercibility, vouched by a prosupport
function", would that be a bad thing?

Regards,
-Chap