Michael-
I got your response and need to sit down and read it carefully. I
strongly wish to continue pursuing this. I completely agree that any
changes should be as broadly useful as possible. I need to go back to the
drawing board.
I'll be in touch.
-Ed
On Wed, Mar 25, 2026 at 12:02 AM Michael Paquier <[email protected]>
wrote:
> On Wed, Feb 11, 2026 at 12:24:31AM +0100, Jelte Fennema-Nio wrote:
> > Yes, it's internally. But to me the whole point of creating an
> > extension (with native code) is so you can access internal things.
> > This is the only type that we're currently hiding the internal
> > structure of, and I don't think numeric is any more special than any
> > of our other complex data types.
>
> This was marked as ready for committer, so I have looked at it to see
> if it would be possible to salvage something for this release, and my
> short answer is no, after looking at this thread and the version of
> the patch posted here:
>
> https://www.postgresql.org/message-id/CAJBL5DOz9=qzaxsagmet46odgcwws3hpy3scbeyqxhhgxqr...@mail.gmail.com
>
> Now, the longer answer. I agree with Robert's feeling on the matter
> that it is a bad experience for extension developers to have to
> copy/paste hundreds if not thousands of in-core code in an extension
> because we forgot the concept of publishing some in-core APIs that
> could be used instead. Regarding the patch, it is absolutely unclear
> to me why this level of copy/paste from numeric.c to numeric.h is the
> right thing to do, with only one routine rename, this one, so as it is
> less generic:
> - * make_result() -
> + * numeric_make_result() -
>
> My opinion is in line with this point from Tom, FWIW:
> https://www.postgresql.org/message-id/[email protected]
>
> We have also that:
>
> +extern void alloc_var(NumericVar *var, int ndigits);
> +extern void free_var(NumericVar *var);
> +extern void zero_var(NumericVar *var);
> +
> +extern void set_var_from_num(Numeric num, NumericVar *dest);
> +extern void init_var_from_num(Numeric num, NumericVar *dest);
> +extern void set_var_from_var(const NumericVar *value, NumericVar *dest);
>
> Var is a concept that exists in the code, so this is just a blind
> unthought move of code from one place to the other, and it is not
> clear at all why and how it would benefit for extensions developers at
> large. It may benefit for the case of the author, but it seems that
> we may want to think about the refactoring and the amount of routines
> to publish at large, around NumericVar.
>
> One suggestion that I would like to offer is to implement a test
> module that has snipets of code one could look at as a start point
> when working on NumericVar, to demonstrate the benefit of this
> refactoring. With none of that presented in the patch, it's a bit
> hard for one to look at these APIs and make sense of how to use
> NumericVar at all. I feel that it may point at something else, could
> it make sense to split numeric.c/h and move some of NumericVar into a
> new set of files? Another thing that would give bonus points for your
> case is test coverage: do we have corner cases related to NumericVar
> that are not stressed now but could gain from this refactoring?
> Having such corner cases in a test module would also help in making an
> argument in favor of copying some or even more of this code.
>
> A last thing to consider is efficiency. Removing some of the
> palloc()/pfree() calls may be interesting on this side, actually,
> numeric can show high in profiles for some workloads. If we can
> improve some typical scenarios on the way, that makes things easier
> to sell, and also perhaps easier to shape.
>
> I am marking this as RwF. I suspect that there are things that we
> could do, but I'd suggest to do more than just a copy/paste to satisfy
> only the case of PL/Haskell you are mentioning. You are obviously
> doing this move for efficiency reasons in your PL code, I get that,
> but this needs more thoughts, with perhaps some API redesign to fit
> not only your case but broader cases that want to play with
> NumericVar, and consider that with a long-term picture in mind.
> --
> Michael
>
