Обсуждение: pgsql: Rename jsonb_replace to jsonb_set and allow it to add new values
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(-)
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
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
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
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
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
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
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
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