Thread
Commits
-
Fix redefinition of typedef Node in numeric.h
- c768637d6c29 17 (unreleased) landed
- 90789900b84a 18 (unreleased) landed
-
Fix jsonpath .decimal() to honor silent mode
- ab35b8d25286 17 (unreleased) landed
- 84001a04d552 18 (unreleased) landed
- 5bbc9b3000a5 19 (unreleased) landed
- 7b12ae729e6a master landed
-
Fix jsonpath .decimal() to honor silent mode
Ewan Young <kdbase.hack@gmail.com> — 2026-07-01T07:52:51Z
Hi The jsonpath .decimal() method handles an invalid precision/scale inconsistently in silent mode, depending only on which internal check happens to catch it: -- precision too large for int4: caught softly, suppressed SELECT jsonb_path_query('1.5', '$.decimal(12345678901)', '{}', true); jsonb_path_query ------------------ (0 rows) -- precision in int4 range but outside NUMERIC's 1..1000: hard error SELECT jsonb_path_query('1.5', '$.decimal(1001)', '{}', true); ERROR: NUMERIC precision 1001 must be between 1 and 1000 Both are the same kind of mistake (a precision the user cannot use), both are in silent mode, yet one returns no rows and the other throws. The same hard error also escapes the @? and @@ operators, which are documented to suppress datetime and numeric errors: SELECT '1.5'::jsonb @? '$.decimal(0)'; ERROR: NUMERIC precision 0 must be between 1 and 1000 .decimal(-1) and .decimal(0) behave like the .decimal(1001) case. The cause is in executeItemOptUnwrapTarget() (jsonpath_exec.c). The precision and scale are converted with numeric_int4_safe() using an ErrorSaveContext, so an int4-overflowing precision is reported softly. But the subsequent typmod construction calls numerictypmodin() through DirectFunctionCall1() with no ErrorSaveContext, so an out-of-range NUMERIC precision/scale throws a hard error that silent mode cannot trap. Every other error path in .decimal() (the numeric_int4_safe() conversions and the numeric_in() value coercion) already reports softly; only the numerictypmodin() call was left bare. This dates back to 66ea94e8e606. This is the same direction taken by 954e57708ea6, which made the sibling jsonpath .split_part() method honor silent mode. The attached patch validates precision and scale against their valid ranges before calling numerictypmodin(), reporting any violation via RETURN_ERROR() so silent mode is honored; after the pre-check numerictypmodin() can no longer throw. In non-silent mode the message becomes method-specific and consistent with the other .decimal() errors: ERROR: precision of jsonpath item method .decimal() must be between 1 and 1000 If reviewers would rather push the ErrorSaveContext down into numerictypmodin() (or a shared helper) instead of pre-validating in the caller, I'm happy to reshape it that way. -- Regards, Ewan Young -
Re: Fix jsonpath .decimal() to honor silent mode
Michael Paquier <michael@paquier.xyz> — 2026-07-01T10:31:39Z
On Wed, Jul 01, 2026 at 03:52:51PM +0800, Ewan Young wrote: > If reviewers would rather push the ErrorSaveContext down into > numerictypmodin() (or a shared helper) > instead of pre-validating in the caller, I'm happy to reshape it that way. My first impression while looking at your patch and the surroundings of numeric.c is that we should do exactly that, and feed from the error message received from the new function to which an escontext is given. That would save in extra error messages (why not just reuse the same errstrings in this case?), and we could rely on error_occurred for the detection. -- Michael
-
Re: Fix jsonpath .decimal() to honor silent mode
Ewan Young <kdbase.hack@gmail.com> — 2026-07-01T12:32:05Z
On Wed, Jul 1, 2026 at 6:31 PM Michael Paquier <michael@paquier.xyz> wrote: > > On Wed, Jul 01, 2026 at 03:52:51PM +0800, Ewan Young wrote: > > If reviewers would rather push the ErrorSaveContext down into > > numerictypmodin() (or a shared helper) > > instead of pre-validating in the caller, I'm happy to reshape it that way. > > My first impression while looking at your patch and the surroundings > of numeric.c is that we should do exactly that, and feed from the > error message received from the new function to which an escontext is > given. That would save in extra error messages (why not just reuse > the same errstrings in this case?), and we could rely on > error_occurred for the detection. Agreed, that's nicer -- v2 attached does it that way. I split numerictypmodin()'s range checks and typmod packing into a new make_numeric_typmod_safe(precision, scale, escontext) in numeric.c. It reports an out-of-range precision or scale through the escontext and returns -1, or throws when escontext is NULL. - numerictypmodin() calls it with a NULL escontext, so its behavior and error messages are unchanged. - .decimal() calls it with its own ErrorSaveContext when not throwing errors (jspThrowErrors(cxt) ? NULL : &escontext) and relies on error_occurred. So there are no new message strings, and in the throwing case the same "NUMERIC precision N must be between 1 and 1000" is raised as before. As a side effect .decimal() no longer needs to build a cstring array to reach numerictypmodin(). Thanks for the review. > -- > Michael -- Regards, Ewan Young
-
Re: Fix jsonpath .decimal() to honor silent mode
Michael Paquier <michael@paquier.xyz> — 2026-07-02T04:50:05Z
On Wed, Jul 01, 2026 at 08:32:05PM +0800, Ewan Young wrote: > Agreed, that's nicer -- v2 attached does it that way. Applied that down to v17, and buildfarm members longfin and sifaka are complaining about a redefinition of Node on v17 and v18: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sifaka&dt=2026-07-02%2004%3A00%3A19 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=longfin&dt=2026-07-02%2004%3A12%3A28 This is 15d7dded0e93 all over again. Double-checking now, if I can.. -- Michael
-
Re: Fix jsonpath .decimal() to honor silent mode
Ewan Young <kdbase.hack@gmail.com> — 2026-07-02T05:27:00Z
On Thu, Jul 2, 2026 at 12:50 PM Michael Paquier <michael@paquier.xyz> wrote: > > On Wed, Jul 01, 2026 at 08:32:05PM +0800, Ewan Young wrote: > > Agreed, that's nicer -- v2 attached does it that way. > > Applied that down to v17, and buildfarm members longfin and sifaka are > complaining about a redefinition of Node on v17 and v18: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sifaka&dt=2026-07-02%2004%3A00%3A19 > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=longfin&dt=2026-07-02%2004%3A12%3A28 > > This is 15d7dded0e93 all over again. Double-checking now, if I can.. Confirmed — sorry for the back-branch breakage. Happy to send a v17/v18 patch (struct Node *escontext, drop the added typedef) if that saves you time, otherwise thanks for taking care of it. > -- > Michael -- Regards, Ewan Young
-
Re: Fix jsonpath .decimal() to honor silent mode
Michael Paquier <michael@paquier.xyz> — 2026-07-02T06:15:06Z
On Thu, Jul 02, 2026 at 01:27:00PM +0800, Ewan Young wrote: > Confirmed — sorry for the back-branch breakage. Happy to send a v17/v18 patch > (struct Node *escontext, drop the added typedef) if that saves you > time, otherwise thanks for > taking care of it. Already fixed with 90789900b84a, hopefully. -- Michael