| From: | Guillaume Lelarge <guillaume(dot)lelarge(at)dalibo(dot)com> |
|---|---|
| To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(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 14:20:27 |
| Message-ID: | be6091c8-5995-4635-a78f-4043aa782f06@dalibo.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2026-11-02 14:20:42 | Re: [PATCH] Corruption Issue: Fix missing tts_tid in ExecForceStoreHeapTuple |
| Previous Message | Nathan Bossart | 2026-11-02 14:14:34 | Re: PoC: true in-place major version upgrades |