Horton

Re: [HACKERS] [PATCH] Generic type subscripting

Dian Fay <dian.m.fay@gmail.com>

View thread View on postgr.es
From: "Dian M Fay" <dian.m.fay@gmail.com>
To: "Pavel Stehule" <pavel.stehule@gmail.com>, "Dmitry Dolgov" <9erthalion6@gmail.com>
Cc: "Tom Lane" <tgl@sss.pgh.pa.us>, "Andres Freund" <andres@anarazel.de>, "Alexander Korotkov" <aekorotkov@gmail.com>, "Justin Pryzby" <pryzby@telsasoft.com>, "David Steele" <david@pgmasters.net>, "Nikita Glukhov" <n.gluhov@postgrespro.ru>, "Alvaro Herrera" <alvherre@2ndquadrant.com>, "David Fetter" <david@fetter.org>, "Thomas Munro" <thomas.munro@gmail.com>, "Oleksandr Shulgin" <oleksandr.shulgin@zalando.de>, "Robert Haas" <robertmhaas@gmail.com>, "Michael Paquier" <michael.paquier@gmail.com>, "Oleg Bartunov" <obartunov@gmail.com>, "Peter Eisentraut" <peter.eisentraut@2ndquadrant.com>, "PostgreSQL Hackers" <pgsql-hackers@postgresql.org>
Date: 2021-01-09T19:02:04Z
Lists: pgsql-hackers 
On Thu Jan 7, 2021 at 3:24 AM EST, Pavel Stehule wrote:
> čt 7. 1. 2021 v 9:15 odesílatel Dmitry Dolgov <9erthalion6@gmail.com>
> napsal:
>
> > > On Wed, Jan 06, 2021 at 09:22:53PM +0100, Pavel Stehule wrote:
> > >
> > > this case should to raise exception - the value should be changed or
> > error
> > > should be raised
> > >
> > > postgres=# insert into foo values('{}');
> > > postgres=# update foo set a['a'] = '100';
> > > postgres=# update foo set a['a'][1] = '-1';
> > > postgres=# select * from foo;
> > > ┌────────────┐
> > > │     a      │
> > > ╞════════════╡
> > > │ {"a": 100} │
> > > └────────────┘
> >
> > I was expecting this question, as I've left this like that intentionally
> > because of two reasons:
> >
> > * Opposite to other changes, to implement this one we need to introduce
> >   a condition more interfering with normal processing, which raises
> >   performance issues for already existing functionality in jsonb_set.
> >
> > * I vaguely recall there was a similar discussion about jsonb_set with
> >   the similar solution.
> >
>
> ok.
>
> In this case I have a strong opinion so current behavior is wrong. It
> can
> mask errors. There are two correct possibilities
>
> 1. raising error - because the update try to apply index on scalar value
>
> 2. replace by array - a = {NULL, -1}
>
> But isn't possible ignore assignment
>
> What do people think about it?

I've been following this thread looking forward to the feature and was
all set to come in on the side of raising an exception here, but then I
tried it in a JS REPL:

; a = {}
{}
; a['a'] = '100'
'100'
; a['a'][1] = -1
-1
; a
{ a: '100' }

; b = {}
{}
; b['b'] = 100
100
; b['b'][1] = -1
-1
; b
{ b: 100 }

Even when the value shouldn't be subscriptable _at all_, the invalid
assignment is ignored silently. But since the patch follows some of
JavaScript's more idiosyncratic leads in other respects (e.g. padding
out arrays with nulls when something is inserted at a higher subscript),
the current behavior makes at least as much sense as JavaScript's
canonical behavior.

There's also the bulk update case to think about. An error makes sense
when there's only one tuple being affected at a time, but with 1000
tuples, should a few no-ops where the JSON turns out to be a structural
mismatch stop the rest from changing correctly? What's the alternative?
The only answer I've got is double-checking the structure in the WHERE
clause, which seems like a lot of effort to go to for something that's
supposed to make working with JSON easier.

Changing the surrounding structure (e.g. turning a['a'] into an array)
seems much more surprising than the no-op, and more likely to have
unforeseen consequences in client code working with the JSON. Ignoring
invalid assignments -- like JavaScript itself -- seems like the best
solution to me.

Commits

  1. Throw error when assigning jsonb scalar instead of a composite object

  2. Filling array gaps during jsonb subscripting

  3. Implementation of subscripting for jsonb

  4. Allow ALTER TYPE to update an existing type's typsubscript value.

  5. Allow subscripting of hstore values.

  6. Support subscripting of arbitrary types, not only arrays.

  7. jit: Reference function pointer types via llvmjit_types.c.

  8. Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

  9. jit: Correct parameter type for generated expression evaluation functions.

  10. Renaming for new subscripting mechanism

  11. Fix assertion failure for SSL connections.

  12. Teach eval_const_expressions() to handle some more cases.