From cdd3c8803b9beaa81145159cfac9e9a6bdd12168 Mon Sep 17 00:00:00 2001 From: crprashant <5108573+crprashant@users.noreply.github.com> Date: Mon, 27 Apr 2026 18:59:23 +0000 Subject: [PATCH 1/4] 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. --- regress/expected/agtype.out | 4 +- regress/expected/expr.out | 114 +++++++++++++++++++++++++++++-- regress/sql/expr.sql | 42 ++++++++++++ src/backend/parser/cypher_gram.y | 8 +++ 4 files changed, 160 insertions(+), 8 deletions(-) diff --git a/regress/expected/agtype.out b/regress/expected/agtype.out index b2f3b83e1..0f4775b88 100644 --- a/regress/expected/agtype.out +++ b/regress/expected/agtype.out @@ -2207,8 +2207,8 @@ SELECT * FROM cypher('orderability_graph', $$ MATCH (n) RETURN n ORDER BY n.prop {"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 {"id": 844424930131980, "label": "vertex", "properties": {"prop": {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge}}::vertex {"id": 844424930131979, "label": "vertex", "properties": {"prop": {"id": 0, "label": "v", "properties": {"i": 0}}::vertex}}::vertex - {"id": 844424930131978, "label": "vertex", "properties": {"prop": {"bool": true}}}::vertex {"id": 844424930131977, "label": "vertex", "properties": {"prop": {"i": 0, "bool": true}}}::vertex + {"id": 844424930131978, "label": "vertex", "properties": {"prop": {"i": null, "bool": true}}}::vertex {"id": 844424930131975, "label": "vertex", "properties": {"prop": [1, 2, 3]}}::vertex {"id": 844424930131976, "label": "vertex", "properties": {"prop": [1, 2, 3, 4, 5]}}::vertex {"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 {"id": 844424930131973, "label": "vertex", "properties": {"prop": "string"}}::vertex {"id": 844424930131976, "label": "vertex", "properties": {"prop": [1, 2, 3, 4, 5]}}::vertex {"id": 844424930131975, "label": "vertex", "properties": {"prop": [1, 2, 3]}}::vertex + {"id": 844424930131978, "label": "vertex", "properties": {"prop": {"i": null, "bool": true}}}::vertex {"id": 844424930131977, "label": "vertex", "properties": {"prop": {"i": 0, "bool": true}}}::vertex - {"id": 844424930131978, "label": "vertex", "properties": {"prop": {"bool": true}}}::vertex {"id": 844424930131979, "label": "vertex", "properties": {"prop": {"id": 0, "label": "v", "properties": {"i": 0}}::vertex}}::vertex {"id": 844424930131980, "label": "vertex", "properties": {"prop": {"id": 2, "label": "e", "end_id": 1, "start_id": 0, "properties": {"i": 0}}::edge}}::vertex {"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 diff --git a/regress/expected/expr.out b/regress/expected/expr.out index 3c80bcae5..d44bc9020 100644 --- a/regress/expected/expr.out +++ b/regress/expected/expr.out @@ -40,18 +40,18 @@ SELECT * FROM cypher('expr', $$RETURN {}$$) AS r(c agtype); SELECT * FROM cypher('expr', $$ RETURN {s: 's', i: 1, f: 1.0, b: true, z: null} $$) AS r(c agtype); - c ------------------------------------------ - {"b": true, "f": 1.0, "i": 1, "s": "s"} + c +---------------------------------------------------- + {"b": true, "f": 1.0, "i": 1, "s": "s", "z": null} (1 row) -- nested maps SELECT * FROM cypher('expr', $$ RETURN {s: {s: 's'}, t: {i: 1, e: {f: 1.0}, s: {a: {b: true}}}, z: null} $$) AS r(c agtype); - c ----------------------------------------------------------------------------- - {"s": {"s": "s"}, "t": {"e": {"f": 1.0}, "i": 1, "s": {"a": {"b": true}}}} + c +--------------------------------------------------------------------------------------- + {"s": {"s": "s"}, "t": {"e": {"f": 1.0}, "i": 1, "s": {"a": {"b": true}}}, "z": null} (1 row) -- @@ -10561,3 +10561,105 @@ NOTICE: graph "list" has been dropped -- -- End of tests -- +-- +-- Issue 2391 - map literals must preserve keys whose values are null +-- +SELECT create_graph('issue_2391'); +NOTICE: graph "issue_2391" has been created + create_graph +-------------- + +(1 row) + +-- single-key null +SELECT * FROM cypher('issue_2391', $$ + RETURN {a: null} AS m +$$) AS (m agtype); + m +------------- + {"a": null} +(1 row) + +-- multiple null values +SELECT * FROM cypher('issue_2391', $$ + RETURN {companyName: null, sinceYear: null} AS m +$$) AS (m agtype); + m +------------------------------------------ + {"sinceYear": null, "companyName": null} +(1 row) + +-- keys() must see the null-valued key +SELECT * FROM cypher('issue_2391', $$ + RETURN keys({a: null}) AS ks +$$) AS (ks agtype); + ks +------- + ["a"] +(1 row) + +-- coalesce passes a non-null map (map itself is not null) through +SELECT * FROM cypher('issue_2391', $$ + RETURN coalesce({a: null}, null) AS m +$$) AS (m agtype); + m +------------- + {"a": null} +(1 row) + +-- nested map values inside an expression also preserve nulls +SELECT * FROM cypher('issue_2391', $$ + RETURN {outer: {inner: null, kept: 1}} AS m +$$) AS (m agtype); + m +--------------------------------------- + {"outer": {"kept": 1, "inner": null}} +(1 row) + +-- mixed non-null and null values are all preserved in order +SELECT * FROM cypher('issue_2391', $$ + RETURN {a: 1, b: null, c: 'x'} AS m +$$) AS (m agtype); + m +------------------------------- + {"a": 1, "b": null, "c": "x"} +(1 row) + +-- control: empty map is still empty +SELECT * FROM cypher('issue_2391', $$ + RETURN {} AS m +$$) AS (m agtype); + m +---- + {} +(1 row) + +-- control: CREATE must still strip top-level null properties so +-- setting a property to null removes it from storage +SELECT * FROM cypher('issue_2391', $$ + CREATE (n:Item {keep: 1, drop: null}) RETURN n +$$) AS (n agtype); + n +----------------------------------------------------------------------------- + {"id": 844424930131969, "label": "Item", "properties": {"keep": 1}}::vertex +(1 row) + +SELECT * FROM cypher('issue_2391', $$ + MATCH (n:Item) RETURN n +$$) AS (n agtype); + n +----------------------------------------------------------------------------- + {"id": 844424930131969, "label": "Item", "properties": {"keep": 1}}::vertex +(1 row) + +SELECT * FROM drop_graph('issue_2391', true); +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table issue_2391._ag_label_vertex +drop cascades to table issue_2391._ag_label_edge +drop cascades to table issue_2391."Item" +NOTICE: graph "issue_2391" has been dropped + drop_graph +------------ + +(1 row) + diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql index b951a2367..fc30743ce 100644 --- a/regress/sql/expr.sql +++ b/regress/sql/expr.sql @@ -4139,3 +4139,45 @@ SELECT * FROM drop_graph('list', true); -- -- End of tests -- + +-- +-- Issue 2391 - map literals must preserve keys whose values are null +-- +SELECT create_graph('issue_2391'); +-- single-key null +SELECT * FROM cypher('issue_2391', $$ + RETURN {a: null} AS m +$$) AS (m agtype); +-- multiple null values +SELECT * FROM cypher('issue_2391', $$ + RETURN {companyName: null, sinceYear: null} AS m +$$) AS (m agtype); +-- keys() must see the null-valued key +SELECT * FROM cypher('issue_2391', $$ + RETURN keys({a: null}) AS ks +$$) AS (ks agtype); +-- coalesce passes a non-null map (map itself is not null) through +SELECT * FROM cypher('issue_2391', $$ + RETURN coalesce({a: null}, null) AS m +$$) AS (m agtype); +-- nested map values inside an expression also preserve nulls +SELECT * FROM cypher('issue_2391', $$ + RETURN {outer: {inner: null, kept: 1}} AS m +$$) AS (m agtype); +-- mixed non-null and null values are all preserved in order +SELECT * FROM cypher('issue_2391', $$ + RETURN {a: 1, b: null, c: 'x'} AS m +$$) AS (m agtype); +-- control: empty map is still empty +SELECT * FROM cypher('issue_2391', $$ + RETURN {} AS m +$$) AS (m agtype); +-- control: CREATE must still strip top-level null properties so +-- setting a property to null removes it from storage +SELECT * FROM cypher('issue_2391', $$ + CREATE (n:Item {keep: 1, drop: null}) RETURN n +$$) AS (n agtype); +SELECT * FROM cypher('issue_2391', $$ + MATCH (n:Item) RETURN n +$$) AS (n agtype); +SELECT * FROM drop_graph('issue_2391', true); diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y index b23fa705a..1bff52569 100644 --- a/src/backend/parser/cypher_gram.y +++ b/src/backend/parser/cypher_gram.y @@ -2118,6 +2118,14 @@ map: n = make_ag_node(cypher_map); n->keyvals = $2; + /* + * By default, a Cypher map literal preserves keys whose + * values are null (openCypher / Neo4j semantics: e.g. + * RETURN {a: null} yields {a: null}, not {}). Callers + * that need property-stripping semantics (CREATE, SET =) + * override this to false in cypher_clause.c. + */ + n->keep_null = true; $$ = (Node *)n; } From 825c121f708580c9a542ef1c99676b36f95430bf Mon Sep 17 00:00:00 2001 From: crprashant <5108573+crprashant@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:25:30 -0700 Subject: [PATCH 2/4] 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> --- regress/expected/expr.out | 8 ++++---- regress/sql/expr.sql | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/regress/expected/expr.out b/regress/expected/expr.out index d44bc9020..25b95f8ec 100644 --- a/regress/expected/expr.out +++ b/regress/expected/expr.out @@ -10558,9 +10558,6 @@ NOTICE: graph "list" has been dropped (1 row) --- --- End of tests --- -- -- Issue 2391 - map literals must preserve keys whose values are null -- @@ -10616,7 +10613,7 @@ $$) AS (m agtype); {"outer": {"kept": 1, "inner": null}} (1 row) --- mixed non-null and null values are all preserved in order +-- mixed non-null and null values are all preserved SELECT * FROM cypher('issue_2391', $$ RETURN {a: 1, b: null, c: 'x'} AS m $$) AS (m agtype); @@ -10663,3 +10660,6 @@ NOTICE: graph "issue_2391" has been dropped (1 row) +-- +-- End of tests +-- diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql index fc30743ce..ca9c234cb 100644 --- a/regress/sql/expr.sql +++ b/regress/sql/expr.sql @@ -4136,10 +4136,6 @@ SELECT * FROM drop_graph('regex', true); SELECT * FROM drop_graph('keys', true); SELECT * FROM drop_graph('list', true); --- --- End of tests --- - -- -- Issue 2391 - map literals must preserve keys whose values are null -- @@ -4164,7 +4160,7 @@ $$) AS (m agtype); SELECT * FROM cypher('issue_2391', $$ RETURN {outer: {inner: null, kept: 1}} AS m $$) AS (m agtype); --- mixed non-null and null values are all preserved in order +-- mixed non-null and null values are all preserved SELECT * FROM cypher('issue_2391', $$ RETURN {a: 1, b: null, c: 'x'} AS m $$) AS (m agtype); @@ -4181,3 +4177,7 @@ SELECT * FROM cypher('issue_2391', $$ MATCH (n:Item) RETURN n $$) AS (n agtype); SELECT * FROM drop_graph('issue_2391', true); + +-- +-- End of tests +-- From c641626e54f2c471dd6b7672329e87f37b3f3e77 Mon Sep 17 00:00:00 2001 From: crprashant <5108573+crprashant@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:56:20 -0700 Subject: [PATCH 3/4] 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> --- regress/expected/expr.out | 8 ++++---- regress/sql/expr.sql | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/regress/expected/expr.out b/regress/expected/expr.out index 25b95f8ec..3cc57d58a 100644 --- a/regress/expected/expr.out +++ b/regress/expected/expr.out @@ -9866,10 +9866,10 @@ SELECT * FROM cypher('accessor_opt', $$ MATCH (n:Person) RETURN {id: id(n), name: n.name} $$) AS (plan agtype); - QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +------------------------------------------------------------------------------------------------------------------------------------ Seq Scan on accessor_opt."Person" n - Output: agtype_build_map_nonull('id'::text, n.id, 'name'::text, agtype_access_operator(VARIADIC ARRAY[n.properties, '"name"'::agtype])) + Output: agtype_build_map('id'::text, n.id, 'name'::text, agtype_access_operator(VARIADIC ARRAY[n.properties, '"name"'::agtype])) (2 rows) SELECT * FROM cypher('accessor_opt', $$ @@ -10662,4 +10662,4 @@ NOTICE: graph "issue_2391" has been dropped -- -- End of tests --- +-- diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql index ca9c234cb..5cced2d93 100644 --- a/regress/sql/expr.sql +++ b/regress/sql/expr.sql @@ -4180,4 +4180,4 @@ SELECT * FROM drop_graph('issue_2391', true); -- -- End of tests --- +-- From 71f8d44d64491db5afb467377065ac4009e7c64e Mon Sep 17 00:00:00 2001 From: crprashant <5108573+crprashant@users.noreply.github.com> Date: Fri, 26 Jun 2026 12:19:07 -0700 Subject: [PATCH 4/4] 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> --- regress/expected/expr.out | 30 +++++++++++++++++++++++++++++- regress/sql/expr.sql | 12 ++++++++++++ src/backend/parser/cypher_gram.y | 8 +++++--- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/regress/expected/expr.out b/regress/expected/expr.out index 3cc57d58a..d6b9ac155 100644 --- a/regress/expected/expr.out +++ b/regress/expected/expr.out @@ -10649,11 +10649,39 @@ $$) AS (n agtype); {"id": 844424930131969, "label": "Item", "properties": {"keep": 1}}::vertex (1 row) +-- nested map values under a write (CREATE / SET =) are preserved: the +-- top-level property map is null-stripped, but a nested map literal is +-- its own node and keeps its null-valued keys +SELECT * FROM cypher('issue_2391', $$ + CREATE (n:Nested {a: {b: null}}) RETURN n +$$) AS (n agtype); + n +--------------------------------------------------------------------------------------- + {"id": 1125899906842625, "label": "Nested", "properties": {"a": {"b": null}}}::vertex +(1 row) + +SELECT * FROM cypher('issue_2391', $$ + MATCH (n:Nested) SET n = {a: {b: null}} RETURN n +$$) AS (n agtype); + n +--------------------------------------------------------------------------------------- + {"id": 1125899906842625, "label": "Nested", "properties": {"a": {"b": null}}}::vertex +(1 row) + +SELECT * FROM cypher('issue_2391', $$ + MATCH (n:Nested) RETURN n +$$) AS (n agtype); + n +--------------------------------------------------------------------------------------- + {"id": 1125899906842625, "label": "Nested", "properties": {"a": {"b": null}}}::vertex +(1 row) + SELECT * FROM drop_graph('issue_2391', true); -NOTICE: drop cascades to 3 other objects +NOTICE: drop cascades to 4 other objects DETAIL: drop cascades to table issue_2391._ag_label_vertex drop cascades to table issue_2391._ag_label_edge drop cascades to table issue_2391."Item" +drop cascades to table issue_2391."Nested" NOTICE: graph "issue_2391" has been dropped drop_graph ------------ diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql index 5cced2d93..251349cef 100644 --- a/regress/sql/expr.sql +++ b/regress/sql/expr.sql @@ -4176,6 +4176,18 @@ $$) AS (n agtype); SELECT * FROM cypher('issue_2391', $$ MATCH (n:Item) RETURN n $$) AS (n agtype); +-- nested map values under a write (CREATE / SET =) are preserved: the +-- top-level property map is null-stripped, but a nested map literal is +-- its own node and keeps its null-valued keys +SELECT * FROM cypher('issue_2391', $$ + CREATE (n:Nested {a: {b: null}}) RETURN n +$$) AS (n agtype); +SELECT * FROM cypher('issue_2391', $$ + MATCH (n:Nested) SET n = {a: {b: null}} RETURN n +$$) AS (n agtype); +SELECT * FROM cypher('issue_2391', $$ + MATCH (n:Nested) RETURN n +$$) AS (n agtype); SELECT * FROM drop_graph('issue_2391', true); -- diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y index 1bff52569..85e588e43 100644 --- a/src/backend/parser/cypher_gram.y +++ b/src/backend/parser/cypher_gram.y @@ -2121,9 +2121,11 @@ map: /* * By default, a Cypher map literal preserves keys whose * values are null (openCypher / Neo4j semantics: e.g. - * RETURN {a: null} yields {a: null}, not {}). Callers - * that need property-stripping semantics (CREATE, SET =) - * override this to false in cypher_clause.c. + * RETURN {a: null} yields {a: null}, not {}). CREATE and + * SET = override this to false on the top-level property + * map in cypher_clause.c so null properties are stripped + * on write; a nested map value is its own node and keeps + * this default, preserving its null-valued keys. */ n->keep_null = true;