Hello,

On 02/11/2026 10:30, Bertrand Drouvot wrote:
Hi,

On Fri, Oct 30, 2026 at 09:19:24PM +0100, Guillaume Lelarge wrote:
On 30/10/2026 20:47, Nathan Bossart wrote:
On Fri, Oct 30, 2026 at 08:41:35AM +0000, Bertrand Drouvot wrote:
On Thu, Oct 29, 2026 at 09:45:54PM +0100, Guillaume Lelarge wrote:
Any comments welcome :)

I did not look at the details but I was a bit skeptical at first, as I wondered
whether this would open the door to similar formatting options for other data
types. Then I did some research and found that 645cb44c5 kind of already opened
the door, so I'm not sure what to think about it.

I'm +1 for the idea.  I've actually wanted something like $subject a handful
of times in the past, usually when trying to format a table to be readable
on the lists.  It'd be nice to not have to amend the query with calls to
round().


Yeah, I can see the use case described here and 645cb44c5 already set a 
precedent
for this.
That's exactly my need. I'm at a customer's office trying to get some
metrics. Having lots of number like 23.12346547987 is not helpful, and makes
it harder to understand the values, especially when the table ends up being
badly formatted (yeah, I know \x).

Anyway, \pset seems the right way to get that kind of feature.

As far the patch, a few comments:

=== 1

+               /* copy the "n_decimals" rest */
+               for (int j = 1; j <= n_decimals ; j++)
+               {
+                       new_str[new_str_pos++] = my_str[i++];
+               }

This can read past my_str and write past new_str. For example:

\pset n_decimals 2
SELECT 1::numeric;

produces:

=================================================================
==3574709==ERROR: AddressSanitizer: heap-buffer-overflow....
WRITE of size 1 at 0x502000008732 thread T0
     #0 0x48621f in format_ndecimals 
/home/postgres/postgresql/postgres/src/fe_utils/print.c:444
     #1 0x4977cb in printQuery 
/home/postgres/postgresql/postgres/src/fe_utils/print.c:3907

on an ASAN enabled build.

Here, only two bytes are allocated for "1". I think the code needs to check the
available fractional digits and compute the required output size before copying.


I think I fixed it. Not sure because I don't have an ASAN enabled build. I have no idea how to get that.

=== 2

postgres=# \pset n_decimals 2
Number of decimals is "2".
postgres=# SELECT 1.239::numeric, 9.999::numeric;
  numeric | numeric
---------+---------
     1.23 |    9.99

This looks like truncation rather than rounding. IIUC the intended behavior is
equivalent to round().


It is definitely a truncation rather than a rounding. Never wanted the complexity of rounding the numbers. I'm fine with the approximation. If you know a simple way to do this, I'm open to add it. But I don't think it's worth the trouble.

=== 3

+       if (n_decimals > 0)
+       {
+               /* add the decimal point */
+               new_str[new_str_pos++] = my_str[i++];

The code assumes that the character after the integer part is a decimal point,
but what about things like?

postgres=# \pset n_decimals 0
Number of decimals is "0".
postgres=# SELECT 1e100::float8;
  float8
--------
       1


I had checked this but didn't find any issue. Probably because I tried small numbers such as 1.2e3, which were converted to 1200. Anyway, I need to fix this. I'll get back to you when I have something.

=== 4

+                               if ((PQftype(result, c) == FLOAT4OID ||
+                                       PQftype(result, c) == FLOAT8OID ||
+                                       PQftype(result, c) == NUMERICOID) &&
+                                       opt->n_decimals > -1)
+                               {
+                                       cell = format_ndecimals(cell, 
opt->n_decimals);
+                               }


format_ndecimals() returns newly allocated memory. Before this patch, replacing
cell in the numericlocale branch was safe because it still pointed into the 
PGresult.

With this patch, the allocation leaks in both cases: when numericlocale is 
disabled,
mustfree remains false. When it is enabled, cell is overwritten by a second
allocation.


Same thing, that needs some work.

=== 5

+                               intval = atoi(value);
+                               if (intval < -1)
+                               {
+                                       intval = -1;
+                               }

atoi() silently accepts malformed values. For example, we could:

postgres=# \pset n_decimals foo
Number of decimals is "0".
postgres=# \pset n_decimals 2toto
Number of decimals is "2".

Worth using ParseVariableNum() here?


Right, this is fixed.

Thanks for your comments. I'll get back when I have a new completely fixed patch.

Regards.


--
Guillaume Lelarge
Consultant
https://dalibo.com


Reply via email to