Re: [HACKERS] [PATCH] Generic type subscripting

From: "Dian M Fay" <dian(dot)m(dot)fay(at)gmail(dot)com>
To: "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com>, "Dmitry Dolgov" <9erthalion6(at)gmail(dot)com>
Cc: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Andres Freund" <andres(at)anarazel(dot)de>, "Alexander Korotkov" <aekorotkov(at)gmail(dot)com>, "Justin Pryzby" <pryzby(at)telsasoft(dot)com>, "David Steele" <david(at)pgmasters(dot)net>, "Nikita Glukhov" <n(dot)gluhov(at)postgrespro(dot)ru>, "Alvaro Herrera" <alvherre(at)2ndquadrant(dot)com>, "David Fetter" <david(at)fetter(dot)org>, "Thomas Munro" <thomas(dot)munro(at)gmail(dot)com>, "Oleksandr Shulgin" <oleksandr(dot)shulgin(at)zalando(dot)de>, "Robert Haas" <robertmhaas(at)gmail(dot)com>, "Michael Paquier" <michael(dot)paquier(at)gmail(dot)com>, "Oleg Bartunov" <obartunov(at)gmail(dot)com>, "Peter Eisentraut" <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "PostgreSQL Hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [PATCH] Generic type subscripting
Date: 2021-01-09 19:02:04
Message-ID: C8EUYXSAKKQZ.38PSCKQZ5LT0O@lamia
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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(at)gmail(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2021-01-09 20:21:59 Use pg_pwrite() in pg_test_fsync
Previous Message Bruce Momjian 2021-01-09 18:17:36 Re: Key management with tests