pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

Started by Andrew Dunstanabout 11 years ago9 messagescomitters

andrew@dunslane.net

about 11 years ago

Rename jsonb_replace to jsonb_set and allow it to add new values

The function is given a fourth parameter, which defaults to true. When
this parameter is true, if the last element of the path is missing
in the original json, jsonb_set creates it in the result and assigns it
the new value. If it is false then the function does nothing unless all
elements of the path are present, including the last.

Based on some original code from Dmitry Dolgov, heavily modified by me.

Catalog version bumped.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/37def4224505f3a23a5eef000f0d05daea59c5b5

Modified Files
--------------
doc/src/sgml/func.sgml | 51 +++++++++--
src/backend/catalog/system_views.sql | 8 ++
src/backend/utils/adt/jsonfuncs.c | 151 +++++++++++++++++++++++----------
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.h | 4 +-
src/include/utils/jsonb.h | 2 +-
src/test/regress/expected/jsonb.out | 124 +++++++++++++++++++++------
src/test/regress/expected/jsonb_1.out | 124 +++++++++++++++++++++------
src/test/regress/sql/jsonb.sql | 46 +++++++---
9 files changed, 385 insertions(+), 127 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Andrew Dunstan (#1)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

Andrew Dunstan <andrew@dunslane.net> writes:

Rename jsonb_replace to jsonb_set and allow it to add new values

Buildfarm doesn't seem terribly pleased with this...

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#2)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

On 05/31/2015 09:40 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Rename jsonb_replace to jsonb_set and allow it to add new values

Buildfarm doesn't seem terribly pleased with this...

Weird. I ran make check, of course, and even on the same machine crake
is on.

Anyway, I'll fix it.

cheers

andrew

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Andrew Dunstan (#3)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

Andrew Dunstan <andrew@dunslane.net> writes:

On 05/31/2015 09:40 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Rename jsonb_replace to jsonb_set and allow it to add new values

Buildfarm doesn't seem terribly pleased with this...

Weird. I ran make check, of course, and even on the same machine crake
is on.

Some of the other machines are showing crashes, not just minor output
diffs.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Tom Lane (#4)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

I wrote:

Some of the other machines are showing crashes, not just minor output
diffs.

I tried HEAD on two different machines, and they are both showing Assert
failures here:

(gdb) bt
#0 0x000000341ce32625 in raise () from /lib64/libc.so.6
#1 0x000000341ce33e05 in abort () from /lib64/libc.so.6
#2 0x00000000007b18f9 in ExceptionalCondition (
conditionName=<value optimized out>, errorType=<value optimized out>,
fileName=<value optimized out>, lineNumber=<value optimized out>)
at assert.c:54
#3 0x000000000072bd2b in pushJsonbValueScalar (pstate=0x7fff149ec388,
seq=<value optimized out>, scalarVal=0x7fff149ec280) at jsonb_util.c:584
#4 0x000000000072c0f7 in pushJsonbValue (pstate=0x7fff149ec388,
seq=<value optimized out>, jbval=<value optimized out>) at jsonb_util.c:528
#5 0x000000000072e1a3 in setPathObject (it=0x7fff149ec390,
path_elems=0x201f9c8, path_nulls=0x201f9e8 "", path_len=1,
st=0x7fff149ec388, level=0, newval=0x1fe9628, create=1 '\001')
at jsonfuncs.c:3724
#6 setPath (it=0x7fff149ec390, path_elems=0x201f9c8, path_nulls=0x201f9e8 "",
path_len=1, st=0x7fff149ec388, level=0, newval=0x1fe9628, create=1 '\001')
at jsonfuncs.c:3682
#7 0x000000000072e912 in jsonb_set (fcinfo=<value optimized out>)
at jsonfuncs.c:3486
#8 0x00000000005d0769 in ExecMakeFunctionResultNoSets (fcache=0x1feacc0,
econtext=0x1feb690, isNull=0x7fff149ec4bf "", isDone=<value optimized out>)
at execQual.c:2018
#9 0x00000000005d12cf in ExecEvalFunc (fcache=0x1feacc0, econtext=0x1feb690,

(gdb) p debug_query_string
$2 = 0x1fa6698 "select jsonb_set('{}','{x}','{\"foo\":123}');"

(gdb) f 3
#3 0x000000000072bd2b in pushJsonbValueScalar (pstate=0x7fff149ec388,
seq=<value optimized out>, scalarVal=0x7fff149ec280) at jsonb_util.c:584
584 Assert(scalarVal->type == jbvString);
(gdb) p *scalarVal
$1 = {type = 33460272, val = {numeric = 0x3400000001, boolean = 1 '\001',
string = {len = 1, val = 0x1fe904c "x"}, array = {nElems = 1,
elems = 0x1fe904c, rawScalar = 0 '\000'}, object = {nPairs = 1,
pairs = 0x1fe904c}, binary = {len = 1, data = 0x1fe904c}}}

Looks like something is forgetting to initialize the type field.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#4)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

On 05/31/2015 10:06 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 05/31/2015 09:40 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Rename jsonb_replace to jsonb_set and allow it to add new values

Buildfarm doesn't seem terribly pleased with this...

Weird. I ran make check, of course, and even on the same machine crake
is on.

Some of the other machines are showing crashes, not just minor output
diffs.

*sigh*

well, that's what we have a buildfarm for.

If I can't find the issue very soon I'll revert it.

cheers

andrew

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Andrew Dunstan (#6)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

Andrew Dunstan <andrew@dunslane.net> writes:

well, that's what we have a buildfarm for.

It looks like this problem is in setPathObject:

setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
int path_len, JsonbParseState **st, int level,
Jsonb *newval, uint32 npairs, bool create)
{
JsonbValue v;
int i;
JsonbValue k;
bool done = false;

if (level >= path_len || path_nulls[level])
done = true;

/* empty object is a special case for create */
if ((npairs == 0) && create && (level == path_len - 1))
{
JsonbValue new = k;

new.val.string.len = VARSIZE_ANY_EXHDR(path_elems[level]);
new.val.string.val = VARDATA_ANY(path_elems[level]);

(void) pushJsonbValue(st, WJB_KEY, &new);

Since "k" is undefined at this point, initializing "new" that way is pure
hogwash. I'm surprised gcc doesn't complain about it.

(BTW, could I lobby to not use "new" as a variable name? lldb gets
confused, probably because "new" is a C++ keyword.)

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#5)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

On 05/31/2015 10:17 PM, Tom Lane wrote:

I wrote:

Some of the other machines are showing crashes, not just minor output
diffs.

I tried HEAD on two different machines, and they are both showing Assert
failures here:

(gdb) bt
#0 0x000000341ce32625 in raise () from /lib64/libc.so.6
#1 0x000000341ce33e05 in abort () from /lib64/libc.so.6
#2 0x00000000007b18f9 in ExceptionalCondition (
conditionName=<value optimized out>, errorType=<value optimized out>,
fileName=<value optimized out>, lineNumber=<value optimized out>)
at assert.c:54
#3 0x000000000072bd2b in pushJsonbValueScalar (pstate=0x7fff149ec388,
seq=<value optimized out>, scalarVal=0x7fff149ec280) at jsonb_util.c:584
#4 0x000000000072c0f7 in pushJsonbValue (pstate=0x7fff149ec388,
seq=<value optimized out>, jbval=<value optimized out>) at jsonb_util.c:528
#5 0x000000000072e1a3 in setPathObject (it=0x7fff149ec390,
path_elems=0x201f9c8, path_nulls=0x201f9e8 "", path_len=1,
st=0x7fff149ec388, level=0, newval=0x1fe9628, create=1 '\001')
at jsonfuncs.c:3724
#6 setPath (it=0x7fff149ec390, path_elems=0x201f9c8, path_nulls=0x201f9e8 "",
path_len=1, st=0x7fff149ec388, level=0, newval=0x1fe9628, create=1 '\001')
at jsonfuncs.c:3682
#7 0x000000000072e912 in jsonb_set (fcinfo=<value optimized out>)
at jsonfuncs.c:3486
#8 0x00000000005d0769 in ExecMakeFunctionResultNoSets (fcache=0x1feacc0,
econtext=0x1feb690, isNull=0x7fff149ec4bf "", isDone=<value optimized out>)
at execQual.c:2018
#9 0x00000000005d12cf in ExecEvalFunc (fcache=0x1feacc0, econtext=0x1feb690,

(gdb) p debug_query_string
$2 = 0x1fa6698 "select jsonb_set('{}','{x}','{\"foo\":123}');"

(gdb) f 3
#3 0x000000000072bd2b in pushJsonbValueScalar (pstate=0x7fff149ec388,
seq=<value optimized out>, scalarVal=0x7fff149ec280) at jsonb_util.c:584
584 Assert(scalarVal->type == jbvString);
(gdb) p *scalarVal
$1 = {type = 33460272, val = {numeric = 0x3400000001, boolean = 1 '\001',
string = {len = 1, val = 0x1fe904c "x"}, array = {nElems = 1,
elems = 0x1fe904c, rawScalar = 0 '\000'}, object = {nPairs = 1,
pairs = 0x1fe904c}, binary = {len = 1, data = 0x1fe904c}}}

Looks like something is forgetting to initialize the type field.

Aha!

I fixed a bug regarding that recently, and now I can't find the fix. But
that gives me enough clues to be able to fix it, thanks.

cheers

andrew

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andrew Dunstan

andrew@dunslane.net

about 11 years ago

In reply to: Tom Lane (#7)

Re: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values

On 05/31/2015 10:25 PM, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

well, that's what we have a buildfarm for.

It looks like this problem is in setPathObject:

setPathObject(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
int path_len, JsonbParseState **st, int level,
Jsonb *newval, uint32 npairs, bool create)
{
JsonbValue v;
int i;
JsonbValue k;
bool done = false;

if (level >= path_len || path_nulls[level])
done = true;

/* empty object is a special case for create */
if ((npairs == 0) && create && (level == path_len - 1))
{
JsonbValue new = k;

new.val.string.len = VARSIZE_ANY_EXHDR(path_elems[level]);
new.val.string.val = VARDATA_ANY(path_elems[level]);

(void) pushJsonbValue(st, WJB_KEY, &new);

Since "k" is undefined at this point, initializing "new" that way is pure
hogwash. I'm surprised gcc doesn't complain about it.

(BTW, could I lobby to not use "new" as a variable name? lldb gets
confused, probably because "new" is a C++ keyword.)

OK, I'll fix these too.

cheers

andrew

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers