| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Guillaume Lelarge <guillaume(dot)lelarge(at)dalibo(dot)com> |
| Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Setting the number of digits after the decimal point with a psql's pset variable |
| Date: | 2026-11-02 09:30:48 |
| Message-ID: | auhYSNhQ47Q8DdSJ@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
=== 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().
=== 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
=== 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.
=== 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?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-11-02 09:32:18 | Re: Fix cumulative WAL statistics double-counting from parallel workers |
| Previous Message | Fujii Masao | 2026-11-02 09:21:05 | Re: Incremental backup accepts a prior backup that extends past a timeline switch |