Fix jsonpath .decimal() to honor silent mode
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
Attachments:
v1-0001-Fix-jsonpath-decimal-to-honor-silent-mode.patchapplication/octet-stream; name=v1-0001-Fix-jsonpath-decimal-to-honor-silent-mode.patchDownload+62-6
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
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
Attachments:
v2-0001-Fix-jsonpath-decimal-to-honor-silent-mode.patchapplication/octet-stream; name=v2-0001-Fix-jsonpath-decimal-to-honor-silent-mode.patchDownload+82-40
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
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%3A28This 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
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