Date: Fri, 22 Nov 2024 19:14:25 +0100
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Andy Fan <zhihuifan1213@163.com>
Cc: David Rowley <dgrowleyml@gmail.com>, 
	Peter Eisentraut <peter@eisentraut.org>, Amit Langote <amitlangote09@gmail.com>, 
	Alvaro Herrera <alvherre@alvh.no-ip.org>, jian he <jian.universality@gmail.com>, 
	Chapman Flack <chap@anastigmatix.net>, pgsql-hackers@lists.postgresql.org
Subject: Re: Extract numeric filed in JSONB more effectively
Message-ID: <cudr6wao4hz2otlycx2z2equkigbjxslxvzqb3ejfc4amroheo@frqbvffcgqnu>
References: <87r0hmvuvr.fsf@163.com>
 <8102ff5b-b156-409e-a48f-e53e63a39b36@eisentraut.org>
 <8734t6c5rh.fsf@163.com>
 <87o7bn7z56.fsf@163.com>
 <875xx197bp.fsf@163.com>
 <87ttk0lgcx.fsf@163.com>
 <CAApHDvqnUONCN54dAXdH678ELK7aH2gyVhh0GjTRtrexPd9YMw@mail.gmail.com>
 <8734m5fua1.fsf@163.com>
 <mmilfltxeqgajpi4zok2v2moudot6bhdmwsqgjgnymz3nvkvng@k3caignaldcp>
 <87a5dx4cfb.fsf@163.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87a5dx4cfb.fsf@163.com>
Archived-At: <https://www.postgresql.org/message-id/cudr6wao4hz2otlycx2z2equkigbjxslxvzqb3ejfc4amroheo%40frqbvffcgqnu>
Precedence: bulk

> On Mon, Nov 18, 2024 at 08:23:52AM GMT, Andy Fan wrote:
>
> >> > I imagined you'd the patch should create a SupportRequestSimplify
> >> > support function for jsonb_numeric() that checks if the input
> >> > expression is an OpExpr with funcid of jsonb_object_field().  All you
> >> > do then is ditch the cast and change the OpExpr to call a new function
> >> > named jsonb_object_field_numeric() which returns the val.numeric
> >> > directly.  Likely the same support function could handle jsonb casts
> >> > to other types too, in which case you'd just call some other function,
> >> > e.g jsonb_object_field_timestamp() or jsonb_object_field_boolean().
> >>
> >> Basically yes. The reason complexity comes when we many operators we
> >> want to optimize AND my patch I want to reduce the number of function
> >> created.
> >>
> >> [...]
> >>
> >> Within the start / finish function, we need to create *7* functions.
> >
> > Any particular reason you want to keep number of functions minimal? Is
> > it just to make the patch smaller? I might be missing something without
> > looking at the implementation in details, but the difference between 10
> > and 7 functions doesn't seem to be significant.
>
> Another reason is for reducing code duplication, writting too many
> similar function looks not good to me. Chapman expressed this idea
> first at [1]. Search "it would make me happy to further reduce some
> of the code" in the message.
>
> Acutally this doesn't make the patch complexer too much.
>
> [1]
> https://www.postgresql.org/message-id/5138c6b5fd239e7ce4e1a4e63826ac27%40anastigmatix.net

It might not make everything too much complex, but e.g. relabeling of
the first argument for a "finish" function into an internal one sounds
strange to me. Maybe there is a way to avoid duplication of the code,
but keep all needed functions in pg_proc?

Btw, sorry to complain about small details, but I find start / finish
naming pattern not quite fitting here. Their main purpose is to extract
/ convert a value, the order in which they are happening is less
relevant.