Thread

Commits

  1. make immutability tests in to_json and to_jsonb complete

  2. Include error location in errors from ComputeIndexAttrs().

  3. SQL/JSON: add standard JSON constructor functions

  1. finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2025-05-19T01:09:18Z

    hi.
    
    somehow, I accidentally saw the TODOs (commits [3]) on jsonb.c and json.c
    for functions: to_json_is_immutable and to_jsonb_is_immutable.
    The attached patch is to finalize these TODOs.
    
    per coverage [1], [2],  there was zero coverage for these two functions.
    so I also added extensive tests on it.
    
    [1] https://coverage.postgresql.org/src/backend/utils/adt/jsonb.c.gcov.html
    [2] https://coverage.postgresql.org/src/backend/utils/adt/json.c.gcov.html
    [3] https://git.postgresql.org/cgit/postgresql.git/commit/?id=7081ac46ace8c459966174400b53418683c9fe5c
    
  2. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2025-05-28T06:55:16Z

    On Mon, May 19, 2025 at 9:09 AM jian he <jian.universality@gmail.com> wrote:
    >
    > hi.
    >
    > somehow, I accidentally saw the TODOs (commits [3]) on jsonb.c and json.c
    > for functions: to_json_is_immutable and to_jsonb_is_immutable.
    > The attached patch is to finalize these TODOs.
    >
    > per coverage [1], [2],  there was zero coverage for these two functions.
    > so I also added extensive tests on it.
    >
    
    I didn't include "utils/rel.h", so v1-0001 won't compile.
    The tests were overly verbose,
    so I removed some unnecessary ones to simplify them.
    
  3. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2025-10-13T01:47:17Z

    hi.
    
    rebase and regress tests changes.
    
  4. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2026-01-08T07:05:06Z

    hi.
    
    rebase due to conflict in
    https://git.postgresql.org/cgit/postgresql.git/commit/?id=ba75f717526cbaa9000b546aac456e43d03aaf53
    
    
    --
    jian
    https://www.enterprisedb.com/
    
  5. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com> — 2026-01-13T11:12:15Z

    Hi Jian,
    
    Thanks for working on this. The patch looks good, but I have a few
    observations:
    
    1. Index creation fails for range types regardless of the base data type:
    
    postgres=# create type range_int as range(subtype=int);
    CREATE TYPE
    postgres=# create table range_table(r1 range_int);
    CREATE TABLE
    postgres=# create index on range_table(json_array(r1 returning jsonb));
    ERROR: functions in index expression must be marked IMMUTABLE
    
    I believe this is because range_out is a stable function. Consequently, the
    following
    code snippet in to_jsonb_is_immutable may be redundant, or should simply
    return
    false without recursing for the sub-type:
    
    + else if (att_typtype == TYPTYPE_RANGE)
    + {
    + to_jsonb_is_immutable(get_range_subtype(typoid), contain_mutable);
    + }
    
    2. Regarding the function names to_jsonb_is_immutable and
    to_json_is_immutable:
    since these do not return a boolean value, "is" may no longer be
    appropriate. Perhaps
    something like to_jsonb_check_mutable would be more accurate?
    
    Best regards,
    Vaibhav Dalvi
    EnterpriseDB
    
    On Thu, Jan 8, 2026 at 12:35 PM jian he <jian.universality@gmail.com> wrote:
    
    > hi.
    >
    > rebase due to conflict in
    >
    > https://git.postgresql.org/cgit/postgresql.git/commit/?id=ba75f717526cbaa9000b546aac456e43d03aaf53
    >
    >
    > --
    > jian
    > https://www.enterprisedb.com/
    >
    
  6. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2026-01-21T08:38:24Z

    On Tue, Jan 13, 2026 at 7:12 PM Vaibhav Dalvi
    <vaibhav.dalvi@enterprisedb.com> wrote:
    >
    > Hi Jian,
    >
    > Thanks for working on this. The patch looks good, but I have a few observations:
    >
    > 1. Index creation fails for range types regardless of the base data type:
    >
    > postgres=# create type range_int as range(subtype=int);
    > CREATE TYPE
    > postgres=# create table range_table(r1 range_int);
    > CREATE TABLE
    > postgres=# create index on range_table(json_array(r1 returning jsonb));
    > ERROR: functions in index expression must be marked IMMUTABLE
    >
    > I believe this is because range_out is a stable function. Consequently, the following
    > code snippet in to_jsonb_is_immutable may be redundant, or should simply return
    > false without recursing for the sub-type:
    >
    > + else if (att_typtype == TYPTYPE_RANGE)
    > + {
    > + to_jsonb_is_immutable(get_range_subtype(typoid), contain_mutable);
    > + }
    >
    
    yech, your observation is right.
    
    create index on range_table((r1::text));
    
    will fail, because pg_proc.provolatile is 's' for function range_out.
    
    but imagine the function range_out marked as immutable, then we would
    need to recurse to the range's subtype.
    also, it makes the logic more complite: if the type is container type,
    recurse to check its elemental/subtype.
    
    Therefore, I am inclined to leave it unchanged.
    
    > 2. Regarding the function names to_jsonb_is_immutable and to_json_is_immutable:
    > since these do not return a boolean value, "is" may no longer be appropriate. Perhaps
    > something like to_jsonb_check_mutable would be more accurate?
    >
    
    to_jsonb_is_immutable and to_json_is_immutable are marked as extern, so they may
    be used by other extensions. Renaming them would require extension authors to
    discover and update to the new symbol names. Adding an additional argument makes
    the change immediately visible through a signature mismatch.
    
    so I think we should stick to the current name.
    
    --
    jian
    https://www.enterprisedb.com
    
    
    
    
  7. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com> — 2026-01-22T04:13:32Z

    Hi Jian,
    
    Thanks for your detailed explanation. This makes sense to me as well.
    
    Regards,
    Vaibhav
    
    On Wed, Jan 21, 2026 at 2:09 PM jian he <jian.universality@gmail.com> wrote:
    
    > On Tue, Jan 13, 2026 at 7:12 PM Vaibhav Dalvi
    > <vaibhav.dalvi@enterprisedb.com> wrote:
    > >
    > > Hi Jian,
    > >
    > > Thanks for working on this. The patch looks good, but I have a few
    > observations:
    > >
    > > 1. Index creation fails for range types regardless of the base data type:
    > >
    > > postgres=# create type range_int as range(subtype=int);
    > > CREATE TYPE
    > > postgres=# create table range_table(r1 range_int);
    > > CREATE TABLE
    > > postgres=# create index on range_table(json_array(r1 returning jsonb));
    > > ERROR: functions in index expression must be marked IMMUTABLE
    > >
    > > I believe this is because range_out is a stable function. Consequently,
    > the following
    > > code snippet in to_jsonb_is_immutable may be redundant, or should simply
    > return
    > > false without recursing for the sub-type:
    > >
    > > + else if (att_typtype == TYPTYPE_RANGE)
    > > + {
    > > + to_jsonb_is_immutable(get_range_subtype(typoid), contain_mutable);
    > > + }
    > >
    >
    > yech, your observation is right.
    >
    > create index on range_table((r1::text));
    >
    > will fail, because pg_proc.provolatile is 's' for function range_out.
    >
    > but imagine the function range_out marked as immutable, then we would
    > need to recurse to the range's subtype.
    > also, it makes the logic more complite: if the type is container type,
    > recurse to check its elemental/subtype.
    >
    > Therefore, I am inclined to leave it unchanged.
    >
    > > 2. Regarding the function names to_jsonb_is_immutable and
    > to_json_is_immutable:
    > > since these do not return a boolean value, "is" may no longer be
    > appropriate. Perhaps
    > > something like to_jsonb_check_mutable would be more accurate?
    > >
    >
    > to_jsonb_is_immutable and to_json_is_immutable are marked as extern, so
    > they may
    > be used by other extensions. Renaming them would require extension authors
    > to
    > discover and update to the new symbol names. Adding an additional argument
    > makes
    > the change immediately visible through a signature mismatch.
    >
    > so I think we should stick to the current name.
    >
    > --
    > jian
    > https://www.enterprisedb.com
    >
    
  8. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    Andrew Dunstan <andrew@dunslane.net> — 2026-03-05T22:09:09Z

    On 2026-01-08 Th 2:05 AM, jian he wrote:
    > hi.
    >
    > rebase due to conflict in
    > https://git.postgresql.org/cgit/postgresql.git/commit/?id=ba75f717526cbaa9000b546aac456e43d03aaf53
    >
    >
    
    Here's a rework of this patch. It preserves the original signature of 
    to_json{b}_is_immutable, and fixes some code duplication. It also uses 
    the typecache to get composite info instead of calling relation_open, 
    supports MultiRange types, and exits early if we made a recursive call 
    (no need for json_categorize_type etc. in these cases).
    
    
    cheers
    
    
    andrew
    
    
    --
    Andrew Dunstan
    EDB: https://www.enterprisedb.com
    
  9. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2026-03-10T06:31:50Z

    On Fri, Mar 6, 2026 at 6:09 AM Andrew Dunstan <andrew@dunslane.net> wrote:
    >
    > Here's a rework of this patch. It preserves the original signature of
    > to_json{b}_is_immutable, and fixes some code duplication. It also uses
    > the typecache to get composite info instead of calling relation_open,
    > supports MultiRange types, and exits early if we made a recursive call
    > (no need for json_categorize_type etc. in these cases).
    >
    
    
    In json_categorize_type, we have:
            case INT2OID:
            case INT4OID:
            case INT8OID:
            case FLOAT4OID:
            case FLOAT8OID:
            case NUMERICOID:
                getTypeOutputInfo(typoid, outfuncoid, &typisvarlena);
                *tcategory = JSONTYPE_NUMERIC;
    
    select proname, provolatile from pg_Proc where proname in ('int8out',
    'int2out', 'int4out', 'float4out', 'float8out', 'numeric_out');
    
       proname   | provolatile
    -------------+-------------
     int2out     | i
     int4out     | i
     float4out   | i
     float8out   | i
     int8out     | i
     numeric_out | i
    (6 rows)
    
    Therefore for JSONTYPE_NUMERIC, has_mutable in json_check_mutability
    will be false.
    I slightly changed the `switch (tcategory)` handling in `json_check_mutability`.
    Make the handling order the same as JsonTypeCategory.
    
    
    Cosmetic change in src/test/regress/sql/sqljson.sql
    --error
    To
    -- error
    
    
    
    --
    jian
    https://www.enterprisedb.com/
    
  10. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    Andrew Dunstan <andrew@dunslane.net> — 2026-03-10T14:12:39Z

    On 2026-03-10 Tu 2:31 AM, jian he wrote:
    > On Fri, Mar 6, 2026 at 6:09 AM Andrew Dunstan <andrew@dunslane.net> wrote:
    >> Here's a rework of this patch. It preserves the original signature of
    >> to_json{b}_is_immutable, and fixes some code duplication. It also uses
    >> the typecache to get composite info instead of calling relation_open,
    >> supports MultiRange types, and exits early if we made a recursive call
    >> (no need for json_categorize_type etc. in these cases).
    >>
    >
    > In json_categorize_type, we have:
    >          case INT2OID:
    >          case INT4OID:
    >          case INT8OID:
    >          case FLOAT4OID:
    >          case FLOAT8OID:
    >          case NUMERICOID:
    >              getTypeOutputInfo(typoid, outfuncoid, &typisvarlena);
    >              *tcategory = JSONTYPE_NUMERIC;
    >
    > select proname, provolatile from pg_Proc where proname in ('int8out',
    > 'int2out', 'int4out', 'float4out', 'float8out', 'numeric_out');
    >
    >     proname   | provolatile
    > -------------+-------------
    >   int2out     | i
    >   int4out     | i
    >   float4out   | i
    >   float8out   | i
    >   int8out     | i
    >   numeric_out | i
    > (6 rows)
    >
    > Therefore for JSONTYPE_NUMERIC, has_mutable in json_check_mutability
    > will be false.
    > I slightly changed the `switch (tcategory)` handling in `json_check_mutability`.
    > Make the handling order the same as JsonTypeCategory.
    >
    >
    > Cosmetic change in src/test/regress/sql/sqljson.sql
    > --error
    > To
    > -- error
    >
    
    
    OK, here's a v7.
    
    
    . added a break in the loop if we found something mutable
    
    . added test for JSON generated columns (was present for JSONB 
    expression indexes but missing for JSON).
    
    . added test block demonstrating that range_int (subtype=int) and 
    multirange_int are now correctly treated as immutable, allowing 
    expression indexes via json_array()
       and json_object()
    
    
    
    cheers
    
    
    andrew
    
    
    --
    Andrew Dunstan
    EDB: https://www.enterprisedb.com
    
  11. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    jian he <jian.universality@gmail.com> — 2026-03-11T01:53:36Z

    On Tue, Mar 10, 2026 at 10:12 PM Andrew Dunstan <andrew@dunslane.net> wrote:
    >
    > OK, here's a v7.
    >
    > . added a break in the loop if we found something mutable
    >
    > . added test for JSON generated columns (was present for JSONB
    > expression indexes but missing for JSON).
    >
    > . added test block demonstrating that range_int (subtype=int) and
    > multirange_int are now correctly treated as immutable, allowing
    > expression indexes via json_array()
    >    and json_object()
    >
    
    V7 looks good to me.
    one minor issue:
    
    +-- JSON_OBJECTAGG, JSON_ARRAYAGG is aggregate function, can not be
    used in index
    +create index xx on test_mutability(json_objectagg(a: b absent on null
    with unique keys returning jsonb));
    +ERROR:  aggregate functions are not allowed in index expressions
    +LINE 1: create index xx on test_mutability(json_objectagg(a: b absen...
    +                                           ^
    
    +ERROR:  aggregate functions are not allowed in index expressions
    This error case was never exercised in regression tests, so adding it
    here should be fine.
    
    "can not" should be "cannot".
    
    
    
    --
    jian
    https://www.enterprisedb.com/
    
    
    
    
  12. Re: finish TODOs in to_json_is_immutable, to_jsonb_is_immutable also add tests on it

    Andrew Dunstan <andrew@dunslane.net> — 2026-03-17T15:32:02Z

    On 2026-03-10 Tu 9:53 PM, jian he wrote:
    > On Tue, Mar 10, 2026 at 10:12 PM Andrew Dunstan <andrew@dunslane.net> wrote:
    >> OK, here's a v7.
    >>
    >> . added a break in the loop if we found something mutable
    >>
    >> . added test for JSON generated columns (was present for JSONB
    >> expression indexes but missing for JSON).
    >>
    >> . added test block demonstrating that range_int (subtype=int) and
    >> multirange_int are now correctly treated as immutable, allowing
    >> expression indexes via json_array()
    >>     and json_object()
    >>
    > V7 looks good to me.
    > one minor issue:
    >
    > +-- JSON_OBJECTAGG, JSON_ARRAYAGG is aggregate function, can not be
    > used in index
    > +create index xx on test_mutability(json_objectagg(a: b absent on null
    > with unique keys returning jsonb));
    > +ERROR:  aggregate functions are not allowed in index expressions
    > +LINE 1: create index xx on test_mutability(json_objectagg(a: b absen...
    > +                                           ^
    >
    > +ERROR:  aggregate functions are not allowed in index expressions
    > This error case was never exercised in regression tests, so adding it
    > here should be fine.
    >
    > "can not" should be "cannot".
    >
    >
    
    Thanks. Pushed.
    
    
    cheers
    
    
    andrew
    
    
    --
    Andrew Dunstan
    EDB: https://www.enterprisedb.com