Michael-
I finally got a chance to sit down with this. I guess I'm a little
unclear on what is needed to move forward.
For one thing, I think the functions I propose exposing should be
renamed to be less generic. (Their names don't currently indicate they are
for numeric types.) This is, of course, very simple.
I copied so much of the code because the NumbericVar type seems mature
and well-implemented. No need to reinvent the wheel. That being said, the
usage of the NumericVar structure and its associated functions is indeed
unclear. I'd be happy to write either an example use case or documentation.
I'm not sure where such a thing should go.
What are your thoughts? You mention an API redesign. Can you be more
specific?
Thanks
-Ed
On Wed, Mar 25, 2026 at 5:47 AM Ed Behn <[email protected]> wrote:
> 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
>>
>
