Re: access numeric data in module - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: access numeric data in module
Date
Msg-id acNsY2QdJcoKw3kF@paquier.xyz
Whole thread Raw
In response to Re: access numeric data in module  (Jelte Fennema-Nio <postgres@jeltef.nl>)
List pgsql-hackers
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=QZaXsAgMEt46OdGcWws3hpy3SCBEyqXhHGXqrz1A@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/2963166.1740846781@sss.pgh.pa.us

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

Attachment

pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Re: Test timings are increasing too fast for cfbot
Next
From: Masahiko Sawada
Date:
Subject: Re: Initial COPY of Logical Replication is too slow