From 6f5e4fe9fc34f9512919b1c8b6a54952ab288640 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sat, 8 Mar 2014 19:16:31 -0800 Subject: [PATCH] Fix contains operator in jsonb GIN support routine Have gin_extract_jsonb() flag boths keys and elements as keys. I believe this is safe, but additional review would be useful. Add some regression testing for this. The same bug exists for GiST, and that isn't fixed here, so the failing regression test adding for the analogous GiST bug earlier continues to fail. --- src/backend/utils/adt/jsonb_gin.c | 19 +++++++++++++------ src/test/regress/expected/jsonb.out | 13 +++++++++++-- src/test/regress/sql/jsonb.sql | 9 +++++++-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/adt/jsonb_gin.c b/src/backend/utils/adt/jsonb_gin.c index 9e15089341..3ffa051c0a 100644 --- a/src/backend/utils/adt/jsonb_gin.c +++ b/src/backend/utils/adt/jsonb_gin.c @@ -64,14 +64,17 @@ gin_extract_jsonb(PG_FUNCTION_ARGS) switch (r) { case WJB_KEY: + /* + * An element of an array is serialized as a key for our + * purposes. This is necessary because array elements are also + * keys. + */ + case WJB_ELEM: entries[i++] = PointerGetDatum(makeitemFromValue(&v, KEYFLAG)); break; case WJB_VALUE: entries[i++] = PointerGetDatum(makeitemFromValue(&v, VALFLAG)); break; - case WJB_ELEM: - entries[i++] = PointerGetDatum(makeitemFromValue(&v, ELEMFLAG)); - break; default: break; } @@ -409,6 +412,11 @@ makeitem(char *str, int len, char flag) return item; } +/* + * Create a textual representation of a jsonbValue for GIN storage. + * + * N.B: This should only be called where the recheck flag will be set. + */ static text * makeitemFromValue(JsonbValue * v, char flag) { @@ -424,11 +432,10 @@ makeitemFromValue(JsonbValue * v, char flag) item = makeitem((v->boolean) ? " t" : " f", 2, flag); break; case jbvNumeric: - /* * A textual, locale and precision independent representation of - * numeric is required. Use the standard hash_numeric for this. - * This is sufficient because the recheck flag will be set anyway. + * numeric is required. Use the standard hash_numeric to build + * one. */ cstr = palloc(8 /* hex numbers */ + 1 /* \0 */ ); snprintf(cstr, 9, "%08x", DatumGetInt32(DirectFunctionCall1(hash_numeric, diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 0c7e4869fe..e7a28f3400 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -1431,7 +1431,7 @@ SELECT count(*) FROM testjsonb WHERE j ?& ARRAY['public','disabled']; 42 (1 row) --- array exists - array elements should behave as keys (for index scans too) +-- array exists - array elements should behave as keys (for GiST index scans too) CREATE INDEX jidx_array ON testjsonb USING gist((j->'array')); SELECT 1 from testjsonb WHERE j->'array' ? 'bar'; ?column? @@ -1492,6 +1492,14 @@ SELECT count(*) FROM testjsonb WHERE j ?& ARRAY['public','disabled']; 42 (1 row) +-- array exists - array elements should behave as keys (for GIN index scans too) +CREATE INDEX jidx_array ON testjsonb USING gin((j->'array')); +SELECT 1 from testjsonb WHERE j->'array' ? 'bar'; + ?column? +---------- + 1 +(1 row) + RESET enable_seqscan; SELECT count(*) FROM (SELECT (jsonb_each(j)).key FROM testjsonb) AS wow; count @@ -1559,8 +1567,9 @@ SELECT distinct * FROM (values (jsonb '{}' || ''),('{}')) v(j); SET enable_sort = on; RESET enable_hashagg; RESET enable_sort; --- btree DROP INDEX jidx; +DROP INDEX jidx_array; +-- btree CREATE INDEX jidx ON testjsonb USING btree (j); SET enable_seqscan = off; SELECT count(*) FROM testjsonb WHERE j > '{"p":1}'; diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index b91ac14e77..384bede765 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -319,7 +319,7 @@ SELECT count(*) FROM testjsonb WHERE j @> '{"age":25.0}'; SELECT count(*) FROM testjsonb WHERE j ? 'public'; SELECT count(*) FROM testjsonb WHERE j ?| ARRAY['public','disabled']; SELECT count(*) FROM testjsonb WHERE j ?& ARRAY['public','disabled']; --- array exists - array elements should behave as keys (for index scans too) +-- array exists - array elements should behave as keys (for GiST index scans too) CREATE INDEX jidx_array ON testjsonb USING gist((j->'array')); SELECT 1 from testjsonb WHERE j->'array' ? 'bar'; @@ -339,6 +339,10 @@ SELECT count(*) FROM testjsonb WHERE j ? 'public'; SELECT count(*) FROM testjsonb WHERE j ?| ARRAY['public','disabled']; SELECT count(*) FROM testjsonb WHERE j ?& ARRAY['public','disabled']; +-- array exists - array elements should behave as keys (for GIN index scans too) +CREATE INDEX jidx_array ON testjsonb USING gin((j->'array')); +SELECT 1 from testjsonb WHERE j->'array' ? 'bar'; + RESET enable_seqscan; SELECT count(*) FROM (SELECT (jsonb_each(j)).key FROM testjsonb) AS wow; @@ -357,8 +361,9 @@ SET enable_sort = on; RESET enable_hashagg; RESET enable_sort; --- btree DROP INDEX jidx; +DROP INDEX jidx_array; +-- btree CREATE INDEX jidx ON testjsonb USING btree (j); SET enable_seqscan = off;