Re: [HACKERS] [PATCH] Generic type subscripting - Mailing list pgsql-hackers

From Dmitry Dolgov
Subject Re: [HACKERS] [PATCH] Generic type subscripting
Date
Msg-id 20201222102531.2clsvuqww6etx3by@localhost
Whole thread Raw
In response to Re: [HACKERS] [PATCH] Generic type subscripting  (Dmitry Dolgov <9erthalion6@gmail.com>)
Responses Re: [HACKERS] [PATCH] Generic type subscripting
List pgsql-hackers
> On Fri, Dec 18, 2020 at 08:59:25PM +0100, Dmitry Dolgov wrote:
> > On Thu, Dec 17, 2020 at 03:29:35PM -0500, Tom Lane wrote:
> > Dmitry Dolgov <9erthalion6@gmail.com> writes:
> > > On Thu, Dec 17, 2020 at 01:49:17PM -0500, Tom Lane wrote:
> > >> We can certainly reconsider the API for the parsing hook if there's
> > >> really a good reason for these to be different types, but it seems
> > >> like that would just be encouraging poor design.
> >
> > > To be more specific, this is the current behaviour (an example from the
> > > tests) and it doesn't seem right:
> >
> > >     =# update test_jsonb_subscript
> > >        set test_json['a'] = 3 where id = 1;
> > >     UPDATE 1
> > >     =# select jsonb_typeof(test_json->'a')
> > >        from test_jsonb_subscript where id = 1;
> > >      jsonb_typeof
> > >      --------------
> > >       string
> >
> >
> > I'm rather inclined to think that the result of subscripting a
> > jsonb (and therefore also the required source type for assignment)
> > should be jsonb, not just text.  In that case, something like
> >     update ... set jsoncol['a'] = 3
> > would fail, because there's no cast from integer to jsonb.  You'd
> > have to write one of
> >     update ... set jsoncol['a'] = '3'
> >     update ... set jsoncol['a'] = '"3"'
> > to clarify how you wanted the input to be interpreted.
> > But that seems like a good thing, just as it is for jsonb_in.
>
> Yep, that makes sense, will go with this idea.

Here is the new version of jsonb subscripting rebased on the committed
infrastructure patch. I hope it will not introduce any confusion with
the previously posted patched in this thread (about alter type subscript
and hstore) as they are independent.

There are few differences from the previous version:

* No limit on number of subscripts for jsonb (as there is no intrinsic
  limitation of this kind for jsonb).

* In case of assignment via subscript now it expects the replace value
  to be of jsonb type.

* Similar to the implementation for arrays, if the source jsonb is NULL,
  it will be replaced by an empty jsonb and the new value will be
  assigned to it. This means:

    =# select * from test_jsonb_subscript where id = 3;
     id | test_json
    ----+-----------
      3 | NULL

    =# update test_jsonb_subscript set test_json['a'] = '1' where id = 3;
    UPDATE 1

    =# select * from test_jsonb_subscript where id = 3;
     id | test_json
    ----+-----------
      3 | {"a": 1}

  and similar:

    =# select * from test_jsonb_subscript where id = 3;
     id | test_json
    ----+-----------
      3 | NULL

    =# update test_jsonb_subscript set test_json[1] = '1' where id = 3;
    UPDATE 1

    =# select * from test_jsonb_subscript where id = 3;
     id | test_json
    ----+-----------
      3 | {"1": 1}

  The latter is probably a bit strange looking, but if there are any concerns
  about this part (and in general about an assignment to jsonb which is NULL)
  of the implementation it could be easily changed.

* There is nothing to address question about distinguishing a regular text
  subscript and jsonpath in the patch yet. I guess the idea would be to save
  the original subscript value type before coercing it into text and allow a
  type specific code to convert it back. But I'll probably do it as a separate
  patch when we finish with this one.

Attachment

pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Reduce the number of special cases to build contrib modules on windows
Next
From: Bharath Rupireddy
Date:
Subject: Re: postgres_fdw - cached connection leaks if the associated user mapping/foreign server is dropped