On Wed, Mar 23, 2016 at 7:48 PM, Vitaly Burovoy <[email protected]> wrote: > On 2016-03-23, Oleg Bartunov <[email protected]> wrote: >> On Wed, Mar 23, 2016 at 6:37 AM, Vitaly Burovoy <[email protected]> >> wrote: >> >>> Hello, Hackers! >>> >>> While I was reviewed a patch with "json_insert" function I found a bug >>> which wasn't connected with the patch and reproduced at master. >>> >>> It claims about non-integer whereas input values are obvious integers >>> and in an allowed range. >>> More testing lead to understanding it appears when numbers length are >>> multiplier of 4: >>> >>> postgres=# select jsonb_set('{"a":[[],1,2,3],"b":[]}', '{"a", 9999}', >>> '"4"'); >>> ERROR: path element at the position 2 is not an integer >>> >> >> Hmm, I see in master >> >> select version(); >> version >> ----------------------------------------------------------------------------------------------------------------- >> PostgreSQL 9.6devel on x86_64-apple-darwin15.4.0, compiled by Apple LLVM >> version 7.3.0 (clang-703.0.29), 64-bit >> (1 row) >> >> select jsonb_set('{"a":[[],1,2,3],"b":[]}', '{"a", 9999}', '"4"'); >> jsonb_set >> ------------------------------------ >> {"a": [[], 1, 2, 3, "4"], "b": []} >> (1 row) > > Yes, I can't reproduce it with "CFLAGS=-O2", but it is still > reproduced with "CFLAGS='-O0 -g3'".
On my old-age laptop (OSX 10.8) I can reproduce the failure as well.
> postgres=# select jsonb_set('{"a":[[],1,2,3],"b":[]}', '{"a", 9999}', '"4"');
> ERROR: path element at the position 2 is not an integer
>
> It depends on memory after the string. In debug mode it always (most
> of the time?) has a garbage (in my case the char '~' following by
> '\x7f' multiple times) there.
>
> I think it is just a question of complexity of reproducing in release,
> not a question whether there is a bug or not.
Er, this is definitely a bug. That's not really a problem I think.
> All the other occurrences of strtol in the file have
> TextDatumGetCString before, except the occurrence in the setPathArray
> function. It seems its type is TEXT (which is not null-terminated),
> not cstring.
- char *c = VARDATA_ANY(path_elems[level]);
+ char *keyptr = VARDATA_ANY(path_elems[level]);
+ int keylen = VARSIZE_ANY_EXHDR(path_elems[level]);
+ char c[20 + 1]; /* int64 = 18446744073709551615 (20
symbols) */
long lindex;
That's ugly. We should actually use TextDatumGetCString because the
index is stored as text here via a Datum, and then it is converted
back to an integer. So I propose instead the simple patch attached
that fixes the failure for me. Could you check if that works for you?
--
Michael
jsonb-set-fix-v1.patch
Description: binary/octet-stream
-- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
