Thread

Commits

  1. Fix redefinition of typedef Node in numeric.h

  2. Fix jsonpath .decimal() to honor silent mode

  1. 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
    
  2. 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
    
  3. 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
    
  4. 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
    
  5. 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
    
    
    
    
  6. 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