Skip to content

Commit 54668e1

Browse files
crprashantCopilot
crprashant
and
Copilot
authored
Preserve null-valued keys in map literals (#2391) (#2412)
* Preserve null-valued keys in map literals (#2391) Map literals such as RETURN {a: null} previously dropped keys whose values were null, producing {} instead of {"a": null}. This diverged from the openCypher / Neo4j semantics where map literals preserve every key the user wrote, including those bound to null. Root cause: cypher_map.keep_null defaulted to false (zero-initialised), so the grammar-produced node fed agtype_build_map_nonull, which strips null entries. Call sites that legitimately need strip-null semantics (CREATE node/edge property maps and SET = assignments) already set keep_null=false explicitly, and the MATCH pattern path sets it to true explicitly. Flipping the grammar default to true therefore only affects the cases that were buggy (bare map expressions and nested map values), and leaves CREATE/SET behaviour unchanged. Two preexisting tests encoded the old buggy output and are updated: expr.out (bare RETURN maps now keep the null value) and agtype.out (a nested map inside an orderability test no longer drops its null entry, shifting one row in the ORDER BY result). Dedicated regression coverage for #2391 is added to regress/sql/expr.sql. * Address review: move 'End of tests' marker and drop order claim - Move 'End of tests' to after the Issue 2391 test block so it marks the actual end of the file. - Remove 'in order' from the mixed-values comment since map key ordering is not guaranteed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update accessor EXPLAIN expected output for null-preserving map literals Map literals now build with agtype_build_map (keep_null) instead of agtype_build_map_nonull, so the accessor-optimization EXPLAIN plan in expr.out must reflect the new function name. Also strip a stray trailing CRLF on the last line of expr.sql/expr.out that leaked a carriage return into the regression output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add nested-map write coverage and clarify top-level strip wording A reviewer noted the keep_null=true default reaches further than the PR summary stated: CREATE / SET = only override keep_null=false on the top-level property map, not recursively. A nested map value is its own cypher_map node, so it now inherits the new default and preserves its null-valued keys (e.g. CREATE (n {a: {b: null}}) stores a -> {"b": null}). - regress/sql/expr.sql, regress/expected/expr.out: pin the nested case under a write with CREATE (n:Nested {a: {b: null}}), MATCH ... SET n = {a: {b: null}}, and a MATCH verify. - src/backend/parser/cypher_gram.y: clarify the map: rule comment to state the CREATE / SET override is top-level only and nested map values keep the null-preserving default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fe48740 commit 54668e1

4 files changed

Lines changed: 205 additions & 11 deletions

File tree

‎regress/expected/agtype.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,8 +2207,8 @@ SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop
22072207
{"id": 844424930131981, "label": "vertex", "properties": {"prop": [{"id": 0, "label": "v", "properties": {"i": 0}}::vertex, {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge, {"id": 1, "label": "v", "properties": {"i": 0}}::vertex]::path}}::vertex
22082208
{"id": 844424930131980, "label": "vertex", "properties": {"prop": {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge}}::vertex
22092209
{"id": 844424930131979, "label": "vertex", "properties": {"prop": {"id": 0, "label": "v", "properties": {"i": 0}}::vertex}}::vertex
2210-
{"id": 844424930131978, "label": "vertex", "properties": {"prop": {"bool": true}}}::vertex
22112210
{"id": 844424930131977, "label": "vertex", "properties": {"prop": {"i": 0, "bool": true}}}::vertex
2211+
{"id": 844424930131978, "label": "vertex", "properties": {"prop": {"i": null, "bool": true}}}::vertex
22122212
{"id": 844424930131975, "label": "vertex", "properties": {"prop": [1, 2, 3]}}::vertex
22132213
{"id": 844424930131976, "label": "vertex", "properties": {"prop": [1, 2, 3, 4, 5]}}::vertex
22142214
{"id": 844424930131973, "label": "vertex", "properties": {"prop": "string"}}::vertex
@@ -2230,8 +2230,8 @@ SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop
22302230
{"id": 844424930131973, "label": "vertex", "properties": {"prop": "string"}}::vertex
22312231
{"id": 844424930131976, "label": "vertex", "properties": {"prop": [1, 2, 3, 4, 5]}}::vertex
22322232
{"id": 844424930131975, "label": "vertex", "properties": {"prop": [1, 2, 3]}}::vertex
2233+
{"id": 844424930131978, "label": "vertex", "properties": {"prop": {"i": null, "bool": true}}}::vertex
22332234
{"id": 844424930131977, "label": "vertex", "properties": {"prop": {"i": 0, "bool": true}}}::vertex
2234-
{"id": 844424930131978, "label": "vertex", "properties": {"prop": {"bool": true}}}::vertex
22352235
{"id": 844424930131979, "label": "vertex", "properties": {"prop": {"id": 0, "label": "v", "properties": {"i": 0}}::vertex}}::vertex
22362236
{"id": 844424930131980, "label": "vertex", "properties": {"prop": {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge}}::vertex
22372237
{"id": 844424930131981, "label": "vertex", "properties": {"prop": [{"id": 0, "label": "v", "properties": {"i": 0}}::vertex, {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge, {"id": 1, "label": "v", "properties": {"i": 0}}::vertex]::path}}::vertex

‎regress/expected/expr.out‎

Lines changed: 139 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,18 @@ SELECT * FROM cypher('expr', $$RETURN {}$$) AS r(c agtype);
4040
SELECT * FROM cypher('expr', $$
4141
RETURN {s: 's', i: 1, f: 1.0, b: true, z: null}
4242
$$) AS r(c agtype);
43-
c
44-
-----------------------------------------
45-
{"b": true, "f": 1.0, "i": 1, "s": "s"}
43+
c
44+
----------------------------------------------------
45+
{"b": true, "f": 1.0, "i": 1, "s": "s", "z": null}
4646
(1 row)
4747

4848
-- nested maps
4949
SELECT * FROM cypher('expr', $$
5050
RETURN {s: {s: 's'}, t: {i: 1, e: {f: 1.0}, s: {a: {b: true}}}, z: null}
5151
$$) AS r(c agtype);
52-
c
53-
----------------------------------------------------------------------------
54-
{"s": {"s": "s"}, "t": {"e": {"f": 1.0}, "i": 1, "s": {"a": {"b": true}}}}
52+
c
53+
---------------------------------------------------------------------------------------
54+
{"s": {"s": "s"}, "t": {"e": {"f": 1.0}, "i": 1, "s": {"a": {"b": true}}}, "z": null}
5555
(1 row)
5656

5757
--
@@ -9866,10 +9866,10 @@ SELECT * FROM cypher('accessor_opt', $$
98669866
MATCH (n:Person)
98679867
RETURN {id: id(n), name: n.name}
98689868
$$) AS (plan agtype);
9869-
QUERY PLAN
9870-
-------------------------------------------------------------------------------------------------------------------------------------------
9869+
QUERY PLAN
9870+
------------------------------------------------------------------------------------------------------------------------------------
98719871
Seq Scan on accessor_opt."Person" n
9872-
Output: agtype_build_map_nonull('id'::text, n.id, 'name'::text, agtype_access_operator(VARIADIC ARRAY[n.properties, '"name"'::agtype]))
9872+
Output: agtype_build_map('id'::text, n.id, 'name'::text, agtype_access_operator(VARIADIC ARRAY[n.properties, '"name"'::agtype]))
98739873
(2 rows)
98749874

98759875
SELECT * FROM cypher('accessor_opt', $$
@@ -10558,6 +10558,136 @@ NOTICE: graph "list" has been dropped
1055810558

1055910559
(1 row)
1056010560

10561+
--
10562+
-- Issue 2391 - map literals must preserve keys whose values are null
10563+
--
10564+
SELECT create_graph('issue_2391');
10565+
NOTICE: graph "issue_2391" has been created
10566+
create_graph
10567+
--------------
10568+
10569+
(1 row)
10570+
10571+
-- single-key null
10572+
SELECT * FROM cypher('issue_2391', $$
10573+
RETURN {a: null} AS m
10574+
$$) AS (m agtype);
10575+
m
10576+
-------------
10577+
{"a": null}
10578+
(1 row)
10579+
10580+
-- multiple null values
10581+
SELECT * FROM cypher('issue_2391', $$
10582+
RETURN {companyName: null, sinceYear: null} AS m
10583+
$$) AS (m agtype);
10584+
m
10585+
------------------------------------------
10586+
{"sinceYear": null, "companyName": null}
10587+
(1 row)
10588+
10589+
-- keys() must see the null-valued key
10590+
SELECT * FROM cypher('issue_2391', $$
10591+
RETURN keys({a: null}) AS ks
10592+
$$) AS (ks agtype);
10593+
ks
10594+
-------
10595+
["a"]
10596+
(1 row)
10597+
10598+
-- coalesce passes a non-null map (map itself is not null) through
10599+
SELECT * FROM cypher('issue_2391', $$
10600+
RETURN coalesce({a: null}, null) AS m
10601+
$$) AS (m agtype);
10602+
m
10603+
-------------
10604+
{"a": null}
10605+
(1 row)
10606+
10607+
-- nested map values inside an expression also preserve nulls
10608+
SELECT * FROM cypher('issue_2391', $$
10609+
RETURN {outer: {inner: null, kept: 1}} AS m
10610+
$$) AS (m agtype);
10611+
m
10612+
---------------------------------------
10613+
{"outer": {"kept": 1, "inner": null}}
10614+
(1 row)
10615+
10616+
-- mixed non-null and null values are all preserved
10617+
SELECT * FROM cypher('issue_2391', $$
10618+
RETURN {a: 1, b: null, c: 'x'} AS m
10619+
$$) AS (m agtype);
10620+
m
10621+
-------------------------------
10622+
{"a": 1, "b": null, "c": "x"}
10623+
(1 row)
10624+
10625+
-- control: empty map is still empty
10626+
SELECT * FROM cypher('issue_2391', $$
10627+
RETURN {} AS m
10628+
$$) AS (m agtype);
10629+
m
10630+
----
10631+
{}
10632+
(1 row)
10633+
10634+
-- control: CREATE must still strip top-level null properties so
10635+
-- setting a property to null removes it from storage
10636+
SELECT * FROM cypher('issue_2391', $$
10637+
CREATE (n:Item {keep: 1, drop: null}) RETURN n
10638+
$$) AS (n agtype);
10639+
n
10640+
-----------------------------------------------------------------------------
10641+
{"id": 844424930131969, "label": "Item", "properties": {"keep": 1}}::vertex
10642+
(1 row)
10643+
10644+
SELECT * FROM cypher('issue_2391', $$
10645+
MATCH (n:Item) RETURN n
10646+
$$) AS (n agtype);
10647+
n
10648+
-----------------------------------------------------------------------------
10649+
{"id": 844424930131969, "label": "Item", "properties": {"keep": 1}}::vertex
10650+
(1 row)
10651+
10652+
-- nested map values under a write (CREATE / SET =) are preserved: the
10653+
-- top-level property map is null-stripped, but a nested map literal is
10654+
-- its own node and keeps its null-valued keys
10655+
SELECT * FROM cypher('issue_2391', $$
10656+
CREATE (n:Nested {a: {b: null}}) RETURN n
10657+
$$) AS (n agtype);
10658+
n
10659+
---------------------------------------------------------------------------------------
10660+
{"id": 1125899906842625, "label": "Nested", "properties": {"a": {"b": null}}}::vertex
10661+
(1 row)
10662+
10663+
SELECT * FROM cypher('issue_2391', $$
10664+
MATCH (n:Nested) SET n = {a: {b: null}} RETURN n
10665+
$$) AS (n agtype);
10666+
n
10667+
---------------------------------------------------------------------------------------
10668+
{"id": 1125899906842625, "label": "Nested", "properties": {"a": {"b": null}}}::vertex
10669+
(1 row)
10670+
10671+
SELECT * FROM cypher('issue_2391', $$
10672+
MATCH (n:Nested) RETURN n
10673+
$$) AS (n agtype);
10674+
n
10675+
---------------------------------------------------------------------------------------
10676+
{"id": 1125899906842625, "label": "Nested", "properties": {"a": {"b": null}}}::vertex
10677+
(1 row)
10678+
10679+
SELECT * FROM drop_graph('issue_2391', true);
10680+
NOTICE: drop cascades to 4 other objects
10681+
DETAIL: drop cascades to table issue_2391._ag_label_vertex
10682+
drop cascades to table issue_2391._ag_label_edge
10683+
drop cascades to table issue_2391."Item"
10684+
drop cascades to table issue_2391."Nested"
10685+
NOTICE: graph "issue_2391" has been dropped
10686+
drop_graph
10687+
------------
10688+
10689+
(1 row)
10690+
1056110691
--
1056210692
-- End of tests
1056310693
--

‎regress/sql/expr.sql‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4136,6 +4136,60 @@ SELECT * FROM drop_graph('regex', true);
41364136
SELECT * FROM drop_graph('keys', true);
41374137
SELECT * FROM drop_graph('list', true);
41384138

4139+
--
4140+
-- Issue 2391 - map literals must preserve keys whose values are null
4141+
--
4142+
SELECT create_graph('issue_2391');
4143+
-- single-key null
4144+
SELECT * FROM cypher('issue_2391', $$
4145+
RETURN {a: null} AS m
4146+
$$) AS (m agtype);
4147+
-- multiple null values
4148+
SELECT * FROM cypher('issue_2391', $$
4149+
RETURN {companyName: null, sinceYear: null} AS m
4150+
$$) AS (m agtype);
4151+
-- keys() must see the null-valued key
4152+
SELECT * FROM cypher('issue_2391', $$
4153+
RETURN keys({a: null}) AS ks
4154+
$$) AS (ks agtype);
4155+
-- coalesce passes a non-null map (map itself is not null) through
4156+
SELECT * FROM cypher('issue_2391', $$
4157+
RETURN coalesce({a: null}, null) AS m
4158+
$$) AS (m agtype);
4159+
-- nested map values inside an expression also preserve nulls
4160+
SELECT * FROM cypher('issue_2391', $$
4161+
RETURN {outer: {inner: null, kept: 1}} AS m
4162+
$$) AS (m agtype);
4163+
-- mixed non-null and null values are all preserved
4164+
SELECT * FROM cypher('issue_2391', $$
4165+
RETURN {a: 1, b: null, c: 'x'} AS m
4166+
$$) AS (m agtype);
4167+
-- control: empty map is still empty
4168+
SELECT * FROM cypher('issue_2391', $$
4169+
RETURN {} AS m
4170+
$$) AS (m agtype);
4171+
-- control: CREATE must still strip top-level null properties so
4172+
-- setting a property to null removes it from storage
4173+
SELECT * FROM cypher('issue_2391', $$
4174+
CREATE (n:Item {keep: 1, drop: null}) RETURN n
4175+
$$) AS (n agtype);
4176+
SELECT * FROM cypher('issue_2391', $$
4177+
MATCH (n:Item) RETURN n
4178+
$$) AS (n agtype);
4179+
-- nested map values under a write (CREATE / SET =) are preserved: the
4180+
-- top-level property map is null-stripped, but a nested map literal is
4181+
-- its own node and keeps its null-valued keys
4182+
SELECT * FROM cypher('issue_2391', $$
4183+
CREATE (n:Nested {a: {b: null}}) RETURN n
4184+
$$) AS (n agtype);
4185+
SELECT * FROM cypher('issue_2391', $$
4186+
MATCH (n:Nested) SET n = {a: {b: null}} RETURN n
4187+
$$) AS (n agtype);
4188+
SELECT * FROM cypher('issue_2391', $$
4189+
MATCH (n:Nested) RETURN n
4190+
$$) AS (n agtype);
4191+
SELECT * FROM drop_graph('issue_2391', true);
4192+
41394193
--
41404194
-- End of tests
41414195
--

‎src/backend/parser/cypher_gram.y‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2127,6 +2127,16 @@ map:
21272127

21282128
n = make_ag_node(cypher_map);
21292129
n->keyvals = $2;
2130+
/*
2131+
* By default, a Cypher map literal preserves keys whose
2132+
* values are null (openCypher / Neo4j semantics: e.g.
2133+
* RETURN {a: null} yields {a: null}, not {}). CREATE and
2134+
* SET = override this to false on the top-level property
2135+
* map in cypher_clause.c so null properties are stripped
2136+
* on write; a nested map value is its own node and keeps
2137+
* this default, preserving its null-valued keys.
2138+
*/
2139+
n->keep_null = true;
21302140

21312141
$$ = (Node *)n;
21322142
}

0 commit comments

Comments
 (0)