Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jul 17, 2015 at 11:37 AM, Robert Haas robertmh...@gmail.com wrote: I am on vacation right now, but I might have some time tomorrow to deal with it. If not, it will be Sunday or Monday when I get to it. Is this still pending? Yes. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jul 9, 2015 at 10:49 AM, Andrew Dunstan and...@dunslane.net wrote: Where are we on this? This is currently a 9.5 release blocker. I am on vacation right now, but I might have some time tomorrow to deal with it. If not, it will be Sunday or Monday when I get to it. Is this still pending? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 07/17/2015 02:37 PM, Robert Haas wrote: On Thu, Jul 9, 2015 at 10:49 AM, Andrew Dunstan and...@dunslane.net wrote: Where are we on this? This is currently a 9.5 release blocker. I am on vacation right now, but I might have some time tomorrow to deal with it. If not, it will be Sunday or Monday when I get to it. Is this still pending? Yes, been tied up a bit unexpectedly this week, am just getting down to it right now. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jul 17, 2015 at 6:20 PM, Andrew Dunstan and...@dunslane.net wrote: OK, I have committed this and updated the open issues list on the wiki. Thanks, Andrew. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 07/17/2015 02:49 PM, Andrew Dunstan wrote: On 07/17/2015 02:37 PM, Robert Haas wrote: On Thu, Jul 9, 2015 at 10:49 AM, Andrew Dunstan and...@dunslane.net wrote: Where are we on this? This is currently a 9.5 release blocker. I am on vacation right now, but I might have some time tomorrow to deal with it. If not, it will be Sunday or Monday when I get to it. Is this still pending? Yes, been tied up a bit unexpectedly this week, am just getting down to it right now. OK, I have committed this and updated the open issues list on the wiki. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 07/09/2015 04:10 AM, Peter Geoghegan wrote: On Mon, Jun 22, 2015 at 6:19 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan and...@dunslane.net wrote: Please submit a patch to adjust the treatment of negative integers in the old functions to be consistent with their treatment in the new functions. i.e. in the range [-n,-1] they should refer to the corresponding element counting from the right. This patch is attached, along with a separate patch which adds a release note compatibility item. Where are we on this? This is currently a 9.5 release blocker. I am on vacation right now, but I might have some time tomorrow to deal with it. If not, it will be Sunday or Monday when I get to it. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Mon, Jun 22, 2015 at 6:19 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan and...@dunslane.net wrote: Please submit a patch to adjust the treatment of negative integers in the old functions to be consistent with their treatment in the new functions. i.e. in the range [-n,-1] they should refer to the corresponding element counting from the right. This patch is attached, along with a separate patch which adds a release note compatibility item. Where are we on this? This is currently a 9.5 release blocker. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan and...@dunslane.net wrote: Please submit a patch to adjust the treatment of negative integers in the old functions to be consistent with their treatment in the new functions. i.e. in the range [-n,-1] they should refer to the corresponding element counting from the right. This patch is attached, along with a separate patch which adds a release note compatibility item. See commit message for full details. Thanks -- Peter Geoghegan From 03372a3611be4a8acbf79024aa3626f5ee4ac369 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan peter.geoghega...@gmail.com Date: Mon, 22 Jun 2015 16:00:54 -0700 Subject: [PATCH 2/2] Release note compatibility item Note that json and jsonb extraction operators no longer consider a negative subscript to be invalid. --- doc/src/sgml/release-9.5.sgml | 11 +++ 1 file changed, 11 insertions(+) diff --git a/doc/src/sgml/release-9.5.sgml b/doc/src/sgml/release-9.5.sgml index 4e2ea45..cbe20ca 100644 --- a/doc/src/sgml/release-9.5.sgml +++ b/doc/src/sgml/release-9.5.sgml @@ -125,6 +125,17 @@ /para /listitem +listitem + para + Allow typejson/ and typejsonb/ extraction operators to + accept negative subscripts, which count from the end of JSON + arrays. Historically, these operators yielded literalNULL/ + in the event of a negative subscript, because negative + subscripts were considered invalid. (Peter Geoghegan, Andrew + Dunstan) + /para +/listitem + /itemizedlist /sect2 -- 1.9.1 From f65b2af71b103b2b5802ac20771e59a285bb4a7f Mon Sep 17 00:00:00 2001 From: Peter Geoghegan peter.geoghega...@gmail.com Date: Sat, 6 Jun 2015 20:07:46 -0700 Subject: [PATCH 1/2] Support JSON negative array subscripts everywhere Previously, there was an inconsistency across json/jsonb operators that operate on datums containing JSON arrays -- only some operators supported negative array count-from-the-end subscripting. Specifically, only a new-to-9.5 jsonb deletion operator had support (the new jsonb - integer operator). This inconsistency seemed likely to be counter-intuitive to users. To fix, allow all places where the user can supply an integer subscript to accept a negative subscript value, including path-orientated operators and functions, as well as other extraction operators. This will need to be called out as an incompatibility in the 9.5 release notes, since it's possible that users are relying on certain established extraction operators changed here yielding NULL in the event of a negative subscript. For the json type, this requires adding a way of cheaply getting the total JSON array element count ahead of time when parsing arrays with a negative subscript involved, necessitating an ad-hoc lex and parse. This is followed by a conversion from a negative subscript to its equivalent positive-wise value using the count. From there on, it's as if a positive-wise value was originally provided. Note that there is still a minor inconsistency here across jsonb deletion operators. Unlike the aforementioned new - deletion operator that accepts an integer on its right hand side, the new #- path orientated deletion variant does not throw an error when it appears like an array subscript (input that could be recognized by as an integer literal) is being used on an object, which is wrong-headed. The reason for not being stricter is that it could be the case that an object pair happens to have a key value that looks like an integer; in general, these two possibilities are impossible to differentiate with rhs path text[] argument elements. However, we still don't allow the #- path-orientated deletion operator to perform array-style subscripting. Rather, we just return the original left operand value in the event of a negative subscript (which seems analogous to how the established jsonb/json # text[] path-orientated operator may yield NULL in the event of an invalid subscript). In passing, make SetArrayPath() stricter about not accepting cases where there is trailing non-numeric garbage bytes rather than a clean NUL byte. This means, for example, that strings like 10e10 are now not accepted as an array subscript of 10 by some new-to-9.5 path-orientated jsonb operators (e.g. the new #- operator). Finally, remove dead code for jsonb subscript deletion; arguably, this should have been done in commit b81c7b409. --- doc/src/sgml/func.sgml| 16 -- src/backend/utils/adt/json.c | 39 + src/backend/utils/adt/jsonfuncs.c | 100 +- src/include/utils/jsonapi.h | 7 +++ src/test/regress/expected/json.out| 14 + src/test/regress/expected/json_1.out | 14 + src/test/regress/expected/jsonb.out | 30 ++ src/test/regress/expected/jsonb_1.out | 30 ++ src/test/regress/sql/json.sql | 5 ++ src/test/regress/sql/jsonb.sql| 5 ++ 10 files changed,
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 6/5/15 3:51 PM, Alvaro Herrera wrote:
Jim Nasby wrote:
On 6/5/15 2:08 PM, Petr Jelinek wrote:
That's a good point, and it won't get any better if/when we add the json
point support in 9.6 since the syntax would be something like select
jsonb '{a:1, b:2, c: {a: 2}}' - '/c/a'; and we will again
silently do nothing. That's going to cause bugs in applications using this.
Yeah, this is a miniature version of the pain I've felt with variant: trying
to get sane casting for a data type that encompasses other types in current
Postgres is essentially impossible.
I'm not sure this is the same problem. But anyway I think requiring
explicit casts in this stuff is a good thing -- relying on implicit
cast to text, when most useful behavior uses other types, seems bad.
I'm not sure I'm following, at least for jsonb. If there's only jsonb -
json_pointer operator, why shouldn't we be able to resolve it? I suspect
part of the answer to that problem is that we need to make the
resolution of unknown smarter, or perhaps somehow configurable.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jun 4, 2015 at 5:43 PM, Peter Geoghegan p...@heroku.com wrote:
BTW, there is a bug here -- strtol() needs additional defenses [1]
(before casting to int):
postgres=# select jsonb_set('[1, 2, 3, 4,
5,6,7,8,9,10,11,12,13,14,15,16,17,18]',
'{9223372036854775806}'::text[], 'Input unsanitized', false) ;
jsonb_set
--
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, Input
unsanitized, 18]
(1 row)
[1]
https://www.securecoding.cert.org/confluence/display/cplusplus/INT06-CPP.+Use+strtol()+or+a+related+function+to+convert+a+string+token+to+an+integer
I attach a fix for this bug. The commit message explains everything.
--
Peter Geoghegan
From 2f2042d93d00f85e52612bd7d7499c3238579d4d Mon Sep 17 00:00:00 2001
From: Peter Geoghegan peter.geoghega...@gmail.com
Date: Fri, 12 Jun 2015 14:55:32 -0700
Subject: [PATCH 1/2] Fix path infrastructure bug affecting jsonb_set()
jsonb_set() and other clients of the setPathArray() utility function
could get spurious results when an array integer subscript is provided
that is not within the range of int.
To fix, ensure that the value returned by strtol() within setPathArray()
is within the range of int; when it isn't, assume an invalid input in
line with existing, similar cases. The path-orientated operators that
appeared in PostgreSQL 9.3 and 9.4 do not call setPathArray(), and
already independently take this precaution, so no change there.
---
src/backend/utils/adt/jsonfuncs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index c14d3f7..13d5b7a 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3814,11 +3814,14 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
if (level path_len !path_nulls[level])
{
char *c = VARDATA_ANY(path_elems[level]);
+ long lindex;
errno = 0;
- idx = (int) strtol(c, badp, 10);
- if (errno != 0 || badp == c)
+ lindex = strtol(c, badp, 10);
+ if (errno != 0 || badp == c || lindex INT_MAX || lindex INT_MIN)
idx = nelems;
+ else
+ idx = lindex;
}
else
idx = nelems;
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jun 12, 2015 at 12:26 PM, Andrew Dunstan and...@dunslane.net wrote:
Here's some code for the count piece of that.
Thanks. I'll look into integrating this with what I have.
BTW, on reflection I'm not so sure about my decision to not touch the
logic within jsonb_delete_idx() (commit b81c7b409). I probably should
have changed it in line with the attached patch as part of that
commit. What do you think?
--
Peter Geoghegan
From 8232f360a0696eb9279c29dfa7464edde726c5ae Mon Sep 17 00:00:00 2001
From: Peter Geoghegan peter.geoghega...@gmail.com
Date: Fri, 5 Jun 2015 13:55:48 -0700
Subject: [PATCH 1/2] Remove dead code for jsonb subscript deletion
---
src/backend/utils/adt/jsonfuncs.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index c14d3f7..3fb8327 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3411,10 +3411,8 @@ jsonb_delete_idx(PG_FUNCTION_ARGS)
it = JsonbIteratorInit(in-root);
r = JsonbIteratorNext(it, v, false);
- if (r == WJB_BEGIN_ARRAY)
- n = v.val.array.nElems;
- else
- n = v.val.object.nPairs;
+ Assert (r == WJB_BEGIN_ARRAY);
+ n = v.val.array.nElems;
if (idx 0)
{
@@ -3431,14 +3429,10 @@ jsonb_delete_idx(PG_FUNCTION_ARGS)
while ((r = JsonbIteratorNext(it, v, true)) != 0)
{
- if (r == WJB_ELEM || r == WJB_KEY)
+ if (r == WJB_ELEM)
{
if (i++ == idx)
- {
-if (r == WJB_KEY)
- JsonbIteratorNext(it, v, true); /* skip value */
continue;
- }
}
res = pushJsonbValue(state, r, r WJB_BEGIN_ARRAY ? v : NULL);
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jun 12, 2015 at 4:31 PM, Andrew Dunstan and...@dunslane.net wrote: OK, pushed, although you'd have to be trying really hard to break this. Still, it's reasonable to defend against. I was trying really hard. :-) Thanks -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/10/2015 04:02 PM, Peter Geoghegan wrote: On Wed, Jun 10, 2015 at 11:48 AM, Andrew Dunstan and...@dunslane.net wrote: Sorry for the delay on this. I've been mostly off the grid, having an all too rare visit from Tom Mr Enum Dunstan, and I misunderstood what you were suggesting, Thank you for working with me to address this. I've been busy with other things the past few days too. Please submit a patch to adjust the treatment of negative integers in the old functions to be consistent with their treatment in the new functions. i.e. in the range [-n,-1] they should refer to the corresponding element counting from the right. I've almost finished that patch. I'm currently blocked on deciding what to do about the old path-orientated operators (# and # for json and jsonb types). It's rather painful to support pushing down negative subscripting there -- maybe we should just not do so for those variants, especially given that they're already notationally inconsistent with the other operators that I'll be updating. What do you think? Maybe I'll come up with a simpler way of making that work by taking a fresh look at it, but haven't done that yet. My current, draft approach to making subscripting work with the json variants (not the jsonb variants) is to use a second get_worker() call in the event of a negative subscript, while making the first such call (the existing get_worker() call) establish the number of top-level array elements. That isn't beautiful, and involves some amount of redundant work, but it's probably better than messing with get_worker() in a more invasive way. Besides, this second get_worker() call only needs to happen in the event of a negative subscript, and I'm only really updating this (that is, updating routines like json_array_element()) to preserve consistency with jsonb. What do you think of that idea? Just took a quick look. My impression is that the jsonb case should be fairly easy. If the index is negative, add JB_ROOT_COUNT(container) to it and use that as the argument to getIthJsonbValueFromContainer(). I agree that the json case looks a bit nasty. Maybe a better approach would be to provide a function that, given a JsonLexContext, returns the number of array elements of the current array. In get_array_start we could call that if the relevant path element is negative and adjust it accordingly. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/05/2015 01:51 PM, Andrew Dunstan wrote: On 06/05/2015 01:39 PM, Peter Geoghegan wrote: On Thu, Jun 4, 2015 at 12:10 PM, Peter Geoghegan p...@heroku.com wrote: But I agree that it's not a great contribution to science, especially since the index will be applied to the list of elements in the somewhat counter-intuitive storage order we use, and we could just raise an error if we try to apply integer delete to an object instead of an array. Cool. Do you want to write a patch, or should I? Also, what about negative array subscripting (making the 9.4-era operator jsonb - integer operator support that for consistency with the new operator jsonb - integer operator)? Should I write the patch? Will you commit it if I do? Please let me know if you want me to write these two patches. Send the first one, I'm still thinking about the second one. Sorry for the delay on this. I've been mostly off the grid, having an all too rare visit from Tom Mr Enum Dunstan, and I misunderstood what you were suggesting, Please submit a patch to adjust the treatment of negative integers in the old functions to be consistent with their treatment in the new functions. i.e. in the range [-n,-1] they should refer to the corresponding element counting from the right. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/05/2015 04:48 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Yeah, Good point. Actually, if my memory serves me correctly (always a dubious bet), the avoidance of that kind of ambiguity is why we introduced the # and # operators in the first place, after going round and round for a while on what the API would look like. I should have remembered that when this came around. Mea culpa. So probably the least invasive change would be to rename the text[] variant operator to something like #- and rename the corresponding function to jsonb_delete_path. Not sure that's a great choice of operator name; consider for example select 4#-1; It's not immediately obvious whether the - is meant as a separate unary minus. There are heuristics in the lexer that try to deal with cases like this, but it doesn't seem like a good plan to double down on such heuristics always doing the right thing. Perhaps we should deprectae operator names ending in -? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jun 5, 2015 at 8:32 , Alvaro Herrera alvhe...@2ndquadrant.com
wrote:
Andrew Dunstan wrote:
'some jsonb value' - '{foo,bar}' is already ambiguous - the RH
operand
could be a single text datum or a text array.
Hmm, but that's not in 9.4, so we can still tweak it if necessary.
Consider this jsonb datum. Nobody in their right mind would have a
key
that looks like a path, I hear you say; yet I'm sure this is going to
happen.
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}, {c,a}:
uh}' ;
jsonb
--
{a: 1, b: 2, c: {a: 2}, {c,a}: uh}
(1 fila)
This seems pretty surprising to me:
-- here, the -(jsonb,text) operator is silently chosen, even though
the
-- right operand looks like an array. And we do the wrong thing.
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' -
'{c,a}';
?column?
---
{a: 1, b: 2, c: {a: 2}}
(1 fila)
-- here, the -(jsonb,text[]) operator is chosen
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' -
_text '{c,a}';
?column?
---
{a: 1, b: 2, c: {}}
(1 fila)
But this seems worse to me, because we silently do nothing:
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' -
'{c,a}';
?column?
---
{a: 1, b: 2, c: {a: 2}}
(1 fila)
I think the first operator can be qualified as dangerous. If you
delete
that one, then it's fine because you can't do that query anymore
because
of the conflict with -(jsonb, int).
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' -
'{c,a}';
ERROR: operator is not unique: jsonb - unknown
LÍNEA 1: ...elect jsonb '{a:1, b:2, c: {a: 2}}' -
'{c,a}'...
^
SUGERENCIA: Could not choose a best candidate operator. You might
need to add explicit type casts.
That's a good point, and it won't get any better if/when we add the
json point support in 9.6 since the syntax would be something like
select jsonb '{a:1, b:2, c: {a: 2}}' - '/c/a'; and we
will again silently do nothing. That's going to cause bugs in
applications using this.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
Andrew Dunstan wrote:
On 06/04/2015 03:16 PM, Alvaro Herrera wrote:
I'm just skimming here, but if a jsonb_path type is being proposed,
perhaps it would be better not to have operators that take text or
text[] as second argument. We can provide that functionality with just
functions. For example, it will be confusing to have
jsonb 'some json value' - '{foo,bar}'
operate too differently from
jsonb 'some json value' - json_path '{foo,bar}'
And it will be a nasty regression to have 9.5 allow
jsonb 'some json value' - '{foo,bar}'
and then have 9.6 error out with ambiguous operator when the json_path
thing is added.
The boat has sailed on this. We have had the # and # operators since 9.3,
i.e. even before we got the operators that Peter wants us to adopt the usage
from, and their right hand operands are text arrays with the same path
semantics.
Well, some boats sailed, but maybe those were different boats. I don't
think we should shut discussion off only because we made some choice or
other in the past. Since we haven't released yet, we can base decisions
on what's the most useful API for users, rather on what got committed in
the initial patch.
'some jsonb value' - '{foo,bar}' is already ambiguous - the RH operand
could be a single text datum or a text array.
Hmm, but that's not in 9.4, so we can still tweak it if necessary.
Consider this jsonb datum. Nobody in their right mind would have a key
that looks like a path, I hear you say; yet I'm sure this is going to
happen.
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}, {c,a}: uh}' ;
jsonb
--
{a: 1, b: 2, c: {a: 2}, {c,a}: uh}
(1 fila)
This seems pretty surprising to me:
-- here, the -(jsonb,text) operator is silently chosen, even though the
-- right operand looks like an array. And we do the wrong thing.
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}';
?column?
---
{a: 1, b: 2, c: {a: 2}}
(1 fila)
-- here, the -(jsonb,text[]) operator is chosen
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - _text '{c,a}';
?column?
---
{a: 1, b: 2, c: {}}
(1 fila)
But this seems worse to me, because we silently do nothing:
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}';
?column?
---
{a: 1, b: 2, c: {a: 2}}
(1 fila)
I think the first operator can be qualified as dangerous. If you delete
that one, then it's fine because you can't do that query anymore because
of the conflict with -(jsonb, int).
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}';
ERROR: operator is not unique: jsonb - unknown
LÍNEA 1: ...elect jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}'...
^
SUGERENCIA: Could not choose a best candidate operator. You might need to add
explicit type casts.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
Jim Nasby wrote:
On 6/5/15 2:08 PM, Petr Jelinek wrote:
That's a good point, and it won't get any better if/when we add the json
point support in 9.6 since the syntax would be something like select
jsonb '{a:1, b:2, c: {a: 2}}' - '/c/a'; and we will again
silently do nothing. That's going to cause bugs in applications using this.
Yeah, this is a miniature version of the pain I've felt with variant: trying
to get sane casting for a data type that encompasses other types in current
Postgres is essentially impossible.
I'm not sure this is the same problem. But anyway I think requiring
explicit casts in this stuff is a good thing -- relying on implicit
cast to text, when most useful behavior uses other types, seems bad.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
Andrew Dunstan and...@dunslane.net writes: Yeah, Good point. Actually, if my memory serves me correctly (always a dubious bet), the avoidance of that kind of ambiguity is why we introduced the # and # operators in the first place, after going round and round for a while on what the API would look like. I should have remembered that when this came around. Mea culpa. So probably the least invasive change would be to rename the text[] variant operator to something like #- and rename the corresponding function to jsonb_delete_path. Not sure that's a great choice of operator name; consider for example select 4#-1; It's not immediately obvious whether the - is meant as a separate unary minus. There are heuristics in the lexer that try to deal with cases like this, but it doesn't seem like a good plan to double down on such heuristics always doing the right thing. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/05/2015 02:32 PM, Alvaro Herrera wrote:
'some jsonb value' - '{foo,bar}' is already ambiguous - the RH operand
could be a single text datum or a text array.
Hmm, but that's not in 9.4, so we can still tweak it if necessary.
Consider this jsonb datum. Nobody in their right mind would have a key
that looks like a path, I hear you say; yet I'm sure this is going to
happen.
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}, {c,a}: uh}' ;
jsonb
--
{a: 1, b: 2, c: {a: 2}, {c,a}: uh}
(1 fila)
This seems pretty surprising to me:
-- here, the -(jsonb,text) operator is silently chosen, even though the
-- right operand looks like an array. And we do the wrong thing.
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}';
?column?
---
{a: 1, b: 2, c: {a: 2}}
(1 fila)
-- here, the -(jsonb,text[]) operator is chosen
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - _text '{c,a}';
?column?
---
{a: 1, b: 2, c: {}}
(1 fila)
But this seems worse to me, because we silently do nothing:
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}';
?column?
---
{a: 1, b: 2, c: {a: 2}}
(1 fila)
I think the first operator can be qualified as dangerous. If you delete
that one, then it's fine because you can't do that query anymore because
of the conflict with -(jsonb, int).
alvherre=# select jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}';
ERROR: operator is not unique: jsonb - unknown
LÍNEA 1: ...elect jsonb '{a:1, b:2, c: {a: 2}}' - '{c,a}'...
^
SUGERENCIA: Could not choose a best candidate operator. You might need to add
explicit type casts.
Yeah, Good point. Actually, if my memory serves me correctly (always a
dubious bet), the avoidance of that kind of ambiguity is why we
introduced the # and # operators in the first place, after going
round and round for a while on what the API would look like. I should
have remembered that when this came around. Mea culpa.
So probably the least invasive change would be to rename the text[]
variant operator to something like #- and rename the corresponding
function to jsonb_delete_path.
We could also decide not to keep an operator at all, on the ground that
we think we'll implement a type that encapsulates json pointer in 9.6,
and just keep the renamed function.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 6/5/15 2:08 PM, Petr Jelinek wrote:
That's a good point, and it won't get any better if/when we add the json
point support in 9.6 since the syntax would be something like select
jsonb '{a:1, b:2, c: {a: 2}}' - '/c/a'; and we will again
silently do nothing. That's going to cause bugs in applications using this.
Yeah, this is a miniature version of the pain I've felt with variant:
trying to get sane casting for a data type that encompasses other types
in current Postgres is essentially impossible. Your only option is to
put implicit or assignment casts in and cross your fingers, or to do
only explicit casts and force the user to cast everything (which is a
PITA). Even a json_pointer type may not help this much unless we have
some way to reliable transform an unknown into a json_pointer.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jun 5, 2015 at 10:51 AM, Andrew Dunstan and...@dunslane.net wrote:
Also, what about negative array subscripting (making the 9.4-era
operator jsonb - integer operator support that for consistency with
the new operator jsonb - integer operator)? Should I write the
patch? Will you commit it if I do?
Send the first one, I'm still thinking about the second one.
The first patch is attached.
Regardless of anything else, I see no reason to delay applying my
documentation patch for operator jsonb - text [1].
Thanks
[1]
http://www.postgresql.org/message-id/cam3swzqfswmi2avi-lun_jbyh-rfhq3-0fm8txpw8olc+v8...@mail.gmail.com
--
Peter Geoghegan
From 6513017eabbd4bdd4980056ed73ca8e3fbe58d1b Mon Sep 17 00:00:00 2001
From: Peter Geoghegan peter.geoghega...@gmail.com
Date: Fri, 5 Jun 2015 13:55:48 -0700
Subject: [PATCH] Desupport jsonb subscript deletion on objects
Supporting deletion of JSON pairs within jsonb objects using an
array-style integer subscript allowed for surprising outcomes. This was
mostly due to the implementation-defined ordering of pairs within
objects for jsonb.
It also seems desirable to make jsonb integer subscript deletion
consistent with the 9.4 era general purpose integer subscripting
operator for jsonb (although that operator returns NULL when an object
is encountered, while we prefer to throw an error).
---
doc/src/sgml/func.sgml| 5 ++--
src/backend/utils/adt/jsonfuncs.c | 5
src/test/regress/expected/jsonb.out | 56 ++-
src/test/regress/expected/jsonb_1.out | 56 ++-
src/test/regress/sql/jsonb.sql| 11 +--
5 files changed, 13 insertions(+), 120 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c6e3540..be0c3d6 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10309,8 +10309,9 @@ table2-mapping
row
entryliteral-/literal/entry
entrytypeinteger/type/entry
-entryDelete the field or element with specified index (Negative
-integers count from the end)/entry
+entryDelete the array element with specified index (Negative
+integers count from the end). Throws an error if top level
+container is not an array./entry
entryliteral'[a, b]'::jsonb - 1 /literal/entry
/row
row
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index f87ba77..c14d3f7 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3400,6 +3400,11 @@ jsonb_delete_idx(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(cannot delete from scalar)));
+ if (JB_ROOT_IS_OBJECT(in))
+ ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg(cannot delete from object using integer subscript)));
+
if (JB_ROOT_COUNT(in) == 0)
PG_RETURN_JSONB(in);
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index 412bf97..e6654d4 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3031,54 +3031,6 @@ select '[a,b,c]'::jsonb - -4;
[a, b, c]
(1 row)
-select '{a:1, b:2, c:3}'::jsonb - 3;
- ?column?
---
- {a: 1, b: 2, c: 3}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - 2;
- ?column?
---
- {a: 1, b: 2}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - 1;
- ?column?
---
- {a: 1, c: 3}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - 0;
- ?column?
---
- {b: 2, c: 3}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - -1;
- ?column?
---
- {a: 1, b: 2}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - -2;
- ?column?
---
- {a: 1, c: 3}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - -3;
- ?column?
---
- {b: 2, c: 3}
-(1 row)
-
-select '{a:1, b:2, c:3}'::jsonb - -4;
- ?column?
---
- {a: 1, b: 2, c: 3}
-(1 row)
-
select jsonb_set('{n:null, a:1, b:[1,2], c:{1:2}, d:{1:[2,3]}}'::jsonb, '{n}', '[1,2,3]');
jsonb_set
--
@@ -3192,12 +3144,8 @@ select '[]'::jsonb - 'a';
select 'a'::jsonb - 1; -- error
ERROR: cannot delete from scalar
-select '{}'::jsonb - 1 ;
- ?column?
---
- {}
-(1 row)
-
+select '{}'::jsonb - 1; -- error
+ERROR: cannot delete from object using integer subscript
select '[]'::jsonb - 1;
?column?
--
diff --git a/src/test/regress/expected/jsonb_1.out b/src/test/regress/expected/jsonb_1.out
index 4ead74b..0a1ec93 100644
--- a/src/test/regress/expected/jsonb_1.out
+++ b/src/test/regress/expected/jsonb_1.out
@@ -3031,54 +3031,6 @@ select '[a,b,c]'::jsonb - -4;
[a, b, c]
(1 row)
-select '{a:1, b:2, c:3}'::jsonb - 3;
-
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Fri, Jun 5, 2015 at 1:05 PM, Andrew Dunstan and...@dunslane.net wrote:
So probably the least invasive change would be to rename the text[] variant
operator to something like #- and rename the corresponding function to
jsonb_delete_path.
We could also decide not to keep an operator at all, on the ground that we
think we'll implement a type that encapsulates json pointer in 9.6, and just
keep the renamed function.
Obviously I prefer the latter option, but the former is still an
improvement. To repeat myself, ambiguities around operators are not
the only problem: It seems no good to me that there is no way to
accomplish an equivalent outcome to that shown below with the
similarly-spelled operator you talk about (that is, the operator
currently spelled operator jsonb - text[]):
postgres=# select '[a, c, a]'::jsonb - 'a';
?column?
--
[c]
(1 row)
With the operator currently spelled operator jsonb - text[], at the
very least you have to do this instead:
postgres=# select '[a, c, a]'::jsonb - '{0}'::text[] - '{1}'::text[];
?column?
--
[c]
(1 row)
If nothing else, these operators are too dissimilar for overloading to
be helpful.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jun 4, 2015 at 12:10 PM, Peter Geoghegan p...@heroku.com wrote:
jsonb_delete() should certainly be able to traverse objects, but it's
much less clear that it should be able to *traverse* arrays (affecting
arrays is a different story, though). That's why I proposed not
supporting traversing arrays with it or with jsonb_set(). This would
also removes the questionable second shadow type system within the
text[] rhs operand too, which seems like a good thing.
Here is a further example of why I find this new shadow type system
for rhs text[] operands to be pretty questionable:
postgres=# select jsonb_set('[1, 2, 3, 4, 5,6,7,8,9,10,11,12]',
'{5e10}'::text[], 'Input unsanitized') ;
jsonb_set
---
[1, 2, 3, 4, 5, Input unsanitized, 7, 8, 9, 10, 11, 12]
(1 row)
BTW, there is a bug here -- strtol() needs additional defenses [1]
(before casting to int):
postgres=# select jsonb_set('[1, 2, 3, 4,
5,6,7,8,9,10,11,12,13,14,15,16,17,18]',
'{9223372036854775806}'::text[], 'Input unsanitized', false) ;
jsonb_set
--
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, Input
unsanitized, 18]
(1 row)
[1]
https://www.securecoding.cert.org/confluence/display/cplusplus/INT06-CPP.+Use+strtol()+or+a+related+function+to+convert+a+string+token+to+an+integer
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/04/2015 03:10 PM, Peter Geoghegan wrote: On Thu, Jun 4, 2015 at 6:43 AM, Andrew Dunstan and...@dunslane.net wrote: I've noticed some more issues with the jsonb documentation, and the new jsonb stuff generally. I didn't set out to give Andrew feedback on the semantics weeks after feature freeze, but unfortunately this feels like another discussion that we need to have now rather than later. Yes, I wish you had raised these issues months ago when this was published. That's the way the process is supposed to work. I also wish that I managed to do that. As you know, I was working overtime to get UPSERT into 9.5 during that period. Finding time to review things is always difficult, and I which I could do more. That's happened to me in the past. My view has generally been that in that case I have missed my chance, and I need to live with what others have done. That seems to me preferable to tearing up any pretense we might have to be following a defined development process. I should point out that I have already gone out of my way to accommodate concerns you expressed extremely late about this set of features, and I have lately indicated another area where we can adjust it to meet your objections. Re-litigating this wholesale seems quite a different kettle of fish, however. Just in case it's not clear: I am not at all happy. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jun 4, 2015 at 5:31 PM, Andrew Dunstan and...@dunslane.net wrote: Just in case it's not clear: I am not at all happy. I've offered to help you with several of the issue I raised; I had intended to offer more help. The issues I raise seem pretty substantive to me. I'm trying to make sure that we don't end up with something bad that we need to live with indefinitely. I have offered you something not far off an everybody wins proposal (i.e. no real loss of functionality), and that was my first proposal. I don't know what more I could do for you. I *am* trying to help. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/04/2015 04:13 PM, David E. Wheeler wrote: On Jun 4, 2015, at 12:16 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I'm just skimming here, but if a jsonb_path type is being proposed, Is this not the purpose of JSQuery? https://code.google.com/p/gwtquery/wiki/JsQuery No, it doesn't seem to have anything at all to do with it. What I suggested would be an implementation of json_pointer - see http://tools.ietf.org/html/rfc6901 cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/04/2015 11:33 AM, Jim Nasby wrote: On 6/4/15 8:43 AM, Andrew Dunstan wrote: You are conflating two different things here, quite pointlessly. The RH operand of ?| is not a path, whereas the RH operand of this - variant is. The fact that they are both text arrays doesn't mean that they should mean the same thing. And this is really the whole problem with the rest of your analysis. Has the idea of a specific json_path datatype been discussed? I feel it would add a lot of clarity to the operators. It would also make it easy to have an array of paths, something that's difficult to do today because a path can be an arbitrary length and arrays don't support that. I actually thought of doing something like that earlier today, although I was thinking of making it an array under the hood - I'm not sure how much call there is for an array of paths. We could probably finesse that. I agree that there is some sense in having such a type, especially if we later want to implement json(b)_patch, see http://tools.ietf.org/html/rfc6902. And if we do we should call the type json_pointer to be consistent with http://tools.ietf.org/html/rfc6901. However, this is certainly not 9.5 material. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jun 4, 2015 at 1:02 PM, Peter Geoghegan p...@heroku.com wrote: I would like these new-to-9.5 deletion operators to work at the top level only, like operator jsonb ? text and operator jsonb ?| text, sharing their idea of a key, __including that string array elements are keys__. We haven't got a containment-style nested delete operator for 9.5, but we can hope for it in the future. In the meantime, you get much of the benefit of that with jsonb_delete(), which *can* support nested deletion. It does so by buying into the operator jsonb ? text idea of a key (including that string array elements are keys), although with a twist: the paths text[] right operand operates at multiple nesting levels (not supporting traversing arrays, as Andrew implemented it, but OTOH adding support for deleting String array elements based on the string alone, useful for tag arrays). If in 9.6 we have something like an operator jsonb @- jsonb operator for containment style deletion, and a 9.5 era operator jsonb - text and operator jsonb - text[] pair of operators for existence style deletion (matching operator jsonb ? text, operating only on the top level), that will be pretty good. The fact that jsonb_delete() will have somewhat bridged the gap nesting-deletion-wise for 9.5 (without being usable through an operator) won't really matter then. I want to keep the twist I described out of any jsonb operators that are shipped, and only use it within functions. To be clear: these two paragraphs are a proposal about how I'd like to change things for 9.5 to make the jsonb operators more consistent than the way things are in the master branch, while still offering nested deletion through a function. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Jun 4, 2015, at 12:16 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I'm just skimming here, but if a jsonb_path type is being proposed, Is this not the purpose of JSQuery? https://code.google.com/p/gwtquery/wiki/JsQuery David smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 6/4/15 8:43 AM, Andrew Dunstan wrote: You are conflating two different things here, quite pointlessly. The RH operand of ?| is not a path, whereas the RH operand of this - variant is. The fact that they are both text arrays doesn't mean that they should mean the same thing. And this is really the whole problem with the rest of your analysis. Has the idea of a specific json_path datatype been discussed? I feel it would add a lot of clarity to the operators. It would also make it easy to have an array of paths, something that's difficult to do today because a path can be an arbitrary length and arrays don't support that. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jun 4, 2015 at 6:43 AM, Andrew Dunstan and...@dunslane.net wrote:
I've noticed some more issues with the jsonb documentation, and the
new jsonb stuff generally. I didn't set out to give Andrew feedback on
the semantics weeks after feature freeze, but unfortunately this feels
like another discussion that we need to have now rather than later.
Yes, I wish you had raised these issues months ago when this was published.
That's the way the process is supposed to work.
I also wish that I managed to do that. As you know, I was working
overtime to get UPSERT into 9.5 during that period. Finding time to
review things is always difficult, and I which I could do more.
operator jsonb - integer
===
I think it was a bad idea to allow array-style removal of object
key/value pairs. ISTM that it implies a level of stability in the
ordering that doesn't make sense. Besides, is it really all that
useful?
But I agree that it's not a great contribution to science, especially since
the index will be applied to the list of elements in the somewhat
counter-intuitive storage order we use, and we could just raise an error if
we try to apply integer delete to an object instead of an array.
Cool. Do you want to write a patch, or should I?
Also, what about negative array subscripting (making the 9.4-era
operator jsonb - integer operator support that for consistency with
the new operator jsonb - integer operator)? Should I write the
patch? Will you commit it if I do?
operator jsonb - text[] (and *nested* deletion more generally)
===
Summary: I think that this operator has many problems, and should be
scraped (although only as an operator). IMV nested deletion should
only be handled by functions, and the way that nested deletion works
in general should be slightly adjusted.
The new operator jsonb - text[] operator is confusingly inconsistent
with:
A) operator jsonb text
What exactly is this? I have no idea what you're talking about.
It's a typo -- I meant operator jsonb - text. The fact that
operator jsonb - text and operator jsonb - text[] diverge in the
way they do seems confusing.
The fact that hstore uses it that way doesn't really concern me. Since
hstore isn't nested it doesn't make a whole lot of sense for it to mean
anything else there.
It seems pretty obvious to me that it makes just as much sense as in
hstore. In hstore, you might want to delete multiple key/value pairs
at once, for exactly the same reason as you might want to with jsonb.
Certainly, you'll also want to support nested deletion with jsonb, but
that's beside the point.
But json(b) is nested, and jsonb - path seems quite a
reasonable treatment, something you're much more likely to want to do than
removeing top level elements in bulk.
Probably true. I think that this interface for nested deletion is
complicated enough and inconsistent enough that I'd rather not have an
operator at all, just a function (so somewhat like jsonb_set() --
jsonb_delete()). That is my main point on operator jsonb - text[];
I think the interface is complicated and inconsistent with everything
else for no good reason.
Regarding nested deletion behavior more generally, consider this
example of how this can work out badly:
postgres=# select jsonb_delete(jsonb_set('[a]', '{5}', 'b'), '{5}') ;
jsonb_delete
--
[a, b]
(1 row)
Here, we're adding and then deleting an array element at offset 5 (the
string b). But the element is never deleted by the outer
jsonb_delete(), because we can't rely on the element actually being
stored at offset 5. Seems a bit fragile.
The behaviour of jsonb_set is pretty explicitly documented. If we wanted to
do something else then we'd have to disable the special meaning given to
negative indices, but that would mean in turn we wouldn't be able to prepend
to an array.
jsonb_delete() should certainly be able to traverse objects, but it's
much less clear that it should be able to *traverse* arrays (affecting
arrays is a different story, though). That's why I proposed not
supporting traversing arrays with it or with jsonb_set(). This would
also removes the questionable second shadow type system within the
text[] rhs operand too, which seems like a good thing.
I think that traversing arrays in nested documents is a rare
requirement, because the ordering within arrays is unstable. If you
already know the ordinal number of the thing you want to nuke, then
you probably have already locked the row, and you might as well
manipulate the JSON using Javascript or Python at that stage.
Making jsonb_delete() buy into the operator jsonb ? text idea of a
key (a thing that it must delete) would also allow jsonb_delete() to
reliably delete particular strings in arrays, which actually does make
a lot of sense (think of arrays of tags). But FWIW it's the
inconsistency that bothers me most.
More importantly, consider the
Re: [HACKERS] Further issues with jsonb semantics, documentation
I'm just skimming here, but if a jsonb_path type is being proposed,
perhaps it would be better not to have operators that take text or
text[] as second argument. We can provide that functionality with just
functions. For example, it will be confusing to have
jsonb 'some json value' - '{foo,bar}'
operate too differently from
jsonb 'some json value' - json_path '{foo,bar}'
And it will be a nasty regression to have 9.5 allow
jsonb 'some json value' - '{foo,bar}'
and then have 9.6 error out with ambiguous operator when the json_path
thing is added.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Thu, Jun 4, 2015 at 12:16 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
I'm just skimming here, but if a jsonb_path type is being proposed,
perhaps it would be better not to have operators that take text or
text[] as second argument. We can provide that functionality with just
functions. For example, it will be confusing to have
jsonb 'some json value' - '{foo,bar}'
operate too differently from
jsonb 'some json value' - json_path '{foo,bar}'
And it will be a nasty regression to have 9.5 allow
jsonb 'some json value' - '{foo,bar}'
and then have 9.6 error out with ambiguous operator when the json_path
thing is added.
Fair point, but FWIW I don't think it'll end up being a new type like
json_path -- it'll just be jsonb, as with containment. I can see there
being an operator that performs deletion in a very similar way to how
operator jsonb @ jsonb performs containment (recall that jsonb
containment is a very JSON-ish flavor of containment).
I would like these new-to-9.5 deletion operators to work at the top
level only, like operator jsonb ? text and operator jsonb ?| text,
sharing their idea of a key, __including that string array elements
are keys__. We haven't got a containment-style nested delete operator
for 9.5, but we can hope for it in the future. In the meantime, you
get much of the benefit of that with jsonb_delete(), which *can*
support nested deletion. It does so by buying into the operator jsonb
? text idea of a key (including that string array elements are keys),
although with a twist: the paths text[] right operand operates at
multiple nesting levels (not supporting traversing arrays, as Andrew
implemented it, but OTOH adding support for deleting String array
elements based on the string alone, useful for tag arrays).
If in 9.6 we have something like an operator jsonb @- jsonb operator
for containment style deletion, and a 9.5 era operator jsonb - text
and operator jsonb - text[] pair of operators for existence style
deletion (matching operator jsonb ? text, operating only on the top
level), that will be pretty good. The fact that jsonb_delete() will
have somewhat bridged the gap nesting-deletion-wise for 9.5 (without
being usable through an operator) won't really matter then. I want to
keep the twist I described out of any jsonb operators that are
shipped, and only use it within functions.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On 06/03/2015 10:02 PM, Peter Geoghegan wrote:
I've noticed some more issues with the jsonb documentation, and the
new jsonb stuff generally. I didn't set out to give Andrew feedback on
the semantics weeks after feature freeze, but unfortunately this feels
like another discussion that we need to have now rather than later.
Yes, I wish you had raised these issues months ago when this was
published. That's the way the process is supposed to work.
operator jsonb - integer
===
Summary: I think that this operator has a problem, but a problem that
can easily be fixed.
I think it was a bad idea to allow array-style removal of object
key/value pairs. ISTM that it implies a level of stability in the
ordering that doesn't make sense. Besides, is it really all that
useful?
The origin of this is nested hstore. Looking at my last version of that
patch, I see:
SELECT 'a=1, b=2, c=3'::hstore - 3;
?column?
a=1, b=2, c=3
(1 row)
But I agree that it's not a great contribution to science, especially
since the index will be applied to the list of elements in the somewhat
counter-intuitive storage order we use, and we could just raise an error
if we try to apply integer delete to an object instead of an array.
operator jsonb - text[] (and *nested* deletion more generally)
===
Summary: I think that this operator has many problems, and should be
scraped (although only as an operator). IMV nested deletion should
only be handled by functions, and the way that nested deletion works
in general should be slightly adjusted.
The new operator jsonb - text[] operator is confusingly inconsistent with:
A) operator jsonb text
What exactly is this? I have no idea what you're talking about.
and:
B) the established operator hstore - text[] operator, since that
operator deletes all key/value pairs that have keys that match any of
the right operand text array values. In contrast, this new operator is
passed as its right operand an array of text elements that constitute
a path (so the order in the rhs text[] operand matters). If the text
element in the rhs text[] operand happens to be what would pass for a
Postgres integer literal, it can be used to traverse lhs array values
through subscripting at that nesting level.
The fact that hstore uses it that way doesn't really concern me. Since
hstore isn't nested it doesn't make a whole lot of sense for it to mean
anything else there. But json(b) is nested, and jsonb - path seems quite
a reasonable treatment, something you're much more likely to want to do
than removeing top level elements in bulk.
Regarding nested deletion behavior more generally, consider this
example of how this can work out badly:
postgres=# select jsonb_delete(jsonb_set('[a]', '{5}', 'b'), '{5}') ;
jsonb_delete
--
[a, b]
(1 row)
Here, we're adding and then deleting an array element at offset 5 (the
string b). But the element is never deleted by the outer
jsonb_delete(), because we can't rely on the element actually being
stored at offset 5. Seems a bit fragile.
The behaviour of jsonb_set is pretty explicitly documented. If we wanted
to do something else then we'd have to disable the special meaning given
to negative indices, but that would mean in turn we wouldn't be able to
prepend to an array.
More importantly, consider the inconsistency with operator jsonb
text (point A above):
postgres=# select '[a]'::jsonb ?| '{a}'::text[]; -- historic/9.4 behavior
?column?
--
t
(1 row)
postgres=# select '[a]'::jsonb - '{a}'::text[]; -- new to 9.5
operator, does not delete!
?column?
--
[a]
(1 row)
You are conflating two different things here, quite pointlessly. The RH
operand of ?| is not a path, whereas the RH operand of this - variant
is. The fact that they are both text arrays doesn't mean that they
should mean the same thing. And this is really the whole problem with
the rest of your analysis.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Further issues with jsonb semantics, documentation
On Wed, Jun 3, 2015 at 7:02 PM, Peter Geoghegan p...@heroku.com wrote:
Consider this case:
postgres=# select '{c:5, a:6, b:7}'::jsonb - 1;
?column?
--
{a: 6, c: 5}
(1 row)
Clearly anyone expecting the value a to be removed here would be in
for a surprise. Moreover, it is inconsistent with the established
behavior of the corresponding array-wise subscript operator:
postgres=# select '{c:5, a:6, b:7}'::jsonb - 1;
?column?
--
[null]
(1 row)
For similar reasons, I think that this inconsistency is unacceptable:
postgres=# select '[a, b, c]'::jsonb - -1;
?column?
[a, b]
(1 row)
postgres=# select '[a, b, c]'::jsonb - -1;
?column?
--
[null]
(1 row)
jsonb now supports Python-style negative subscripting to index
backward. I think that this a fine idea. However, I also think it's a
big POLA violation that this was not done for the ordinary array
subscripting operator (operator jsonb - integer) at the same time
as operator jsonb - integer was added. Although doing this will
require a compatibility note in the 9.5 release notes, it's extremely
unlikely to destabilize anybody's app, and makes a lot of sense.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Further issues with jsonb semantics, documentation
I've noticed some more issues with the jsonb documentation, and the
new jsonb stuff generally. I didn't set out to give Andrew feedback on
the semantics weeks after feature freeze, but unfortunately this feels
like another discussion that we need to have now rather than later.
operator jsonb - integer
===
Summary: I think that this operator has a problem, but a problem that
can easily be fixed.
I think it was a bad idea to allow array-style removal of object
key/value pairs. ISTM that it implies a level of stability in the
ordering that doesn't make sense. Besides, is it really all that
useful?
Consider this case:
postgres=# select '{c:5, a:6, b:7}'::jsonb - 1;
?column?
--
{a: 6, c: 5}
(1 row)
Clearly anyone expecting the value a to be removed here would be in
for a surprise. Moreover, it is inconsistent with the established
behavior of the corresponding array-wise subscript operator:
postgres=# select '{c:5, a:6, b:7}'::jsonb - 1;
?column?
--
[null]
(1 row)
I suggest, given that this is conceptually a data-modifying operator,
that the minus operator/jsonb_delete() case raise an error rather than
matching operator jsonb - integer and returning NULL. I say this as
the person who successfully argued that the - operator case above
should return NULL during the 9.4 beta period; returning SQL NULL for
the delete/minus operator feels like going too far in the direction of
permissiveness, even for jsonb; my expression index argument does not
apply here as it did for the operator jsonb - integer case.
operator jsonb - text
Summary: I think that this operator is fine.
Documentation needs work, though. The operator jsonb - text operator
ought to be documented as in the attached patch, which is closer to
the equivalent hstore operator, and emphasizes the operator jsonb ?
text definition of a key. It should emphasize its similarity to the
established operator jsonb ? text operator, and in particular that
array elements behave as keys *iff* they're strings.
operator jsonb - text[] (and *nested* deletion more generally)
===
Summary: I think that this operator has many problems, and should be
scraped (although only as an operator). IMV nested deletion should
only be handled by functions, and the way that nested deletion works
in general should be slightly adjusted.
The new operator jsonb - text[] operator is confusingly inconsistent with:
A) operator jsonb text
and:
B) the established operator hstore - text[] operator, since that
operator deletes all key/value pairs that have keys that match any of
the right operand text array values. In contrast, this new operator is
passed as its right operand an array of text elements that constitute
a path (so the order in the rhs text[] operand matters). If the text
element in the rhs text[] operand happens to be what would pass for a
Postgres integer literal, it can be used to traverse lhs array values
through subscripting at that nesting level.
Regarding nested deletion behavior more generally, consider this
example of how this can work out badly:
postgres=# select jsonb_delete(jsonb_set('[a]', '{5}', 'b'), '{5}') ;
jsonb_delete
--
[a, b]
(1 row)
Here, we're adding and then deleting an array element at offset 5 (the
string b). But the element is never deleted by the outer
jsonb_delete(), because we can't rely on the element actually being
stored at offset 5. Seems a bit fragile.
More importantly, consider the inconsistency with operator jsonb
text (point A above):
postgres=# select '[a]'::jsonb ?| '{a}'::text[]; -- historic/9.4 behavior
?column?
--
t
(1 row)
postgres=# select '[a]'::jsonb - '{a}'::text[]; -- new to 9.5
operator, does not delete!
?column?
--
[a]
(1 row)
Perhaps most questionably of all, the non-array based minus/delete
operator (which I like) *does* have the same idea of matching a key as
the established operator jsonb ?| text[] operator (and operator
jsonb ? text, etc):
postgres=# select '[a]'::jsonb - 'a'::text; -- new to 9.5 operator,
*does* delete!
?column?
--
[]
(1 row)
This conceptual model for manipulating jsonb is entirely new and novel
to this new operator operator text[] (and jsonb_set()).
operator jsonb - text[] categorization/conceptual model
==
Operators like the established operator jsonb - integer operator (a
jsonb array-wise operator) always seemed okay to me because the rhs
operand really was a Postgres integer, and because it's explicitly an
array-wise operator (just like operator - text is explicitly
object-wise). But now, with these new operators, you've added a
shadow type system to certain rhs text[] operands, consisting of
types not explicitly delineated by JSON-style double quotes (for
strings, say). So there is kind of a second shadow type system in
play, similar to that of jsonb except that text[] shadow types
