Preserve null-valued keys in map literals (#2391)

[crprashant]

Summary

Fixes #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.

Changes

• src/backend/parser/cypher_gram.y — the map: rule now sets n->keep_null = true with an explanatory comment.
• regress/sql/expr.sql — dedicated regression coverage for Map literals may drop keys whose values are null. #2391 (single-key null, multiple nulls, keys(), coalesce, nested map values, mixed null / non-null, empty map, CREATE control, MATCH verify).
• regress/expected/expr.out — two preexisting tests that encoded the old buggy output now show the preserved "z": null; plus expected output for the new Map literals may drop keys whose values are null. #2391 block.
• regress/expected/agtype.out — nested {bool: true, i: null} inside an orderability test no longer drops its null entry, shifting one row in the ORDER BY result.

Behaviour before / after

RETURN {a: null} AS m
-- before: {}
-- after:  {"a": null}

RETURN keys({a: null}) AS ks
-- before: []
-- after:  ["a"]

CREATE and SET = still strip null-valued keys (unchanged), matching openCypher semantics for property writes.

Test results

make installcheck against PostgreSQL 18: 32 / 33 pass. The only failure is age_upgrade, which is a preexisting failure unrelated to this change.

[crprashant]

Hi @jrgemignani , @MuhammadTahaNaveed could you pls review this..

[jrgemignani]

Hi @jrgemignani , @MuhammadTahaNaveed could you pls review this..

Sorry for the delay

[Copilot]

Pull request overview

This PR fixes Cypher map literal semantics so that keys with null values are preserved (e.g., RETURN {a: null} returns {"a": null} instead of {}), aligning AGE behavior with openCypher/Neo4j.

Changes:

• Set the grammar-created cypher_map nodes to preserve null values by default (keep_null = true).
• Add regression tests covering null-valued map entries (including nested maps and keys()/coalesce() behavior), plus controls for CREATE property stripping.
• Update expected regression outputs impacted by the corrected semantics (including an orderability test containing a nested map with a null entry).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/backend/parser/cypher_gram.y	Changes map literal default to preserve null values via keep_null = true.
regress/sql/expr.sql	Adds dedicated regression coverage for Issue #2391 and controls for CREATE behavior.
regress/expected/expr.out	Updates expected outputs for preserved null keys and new Issue #2391 test block.
regress/expected/agtype.out	Updates orderability expected output affected by preserved nested null map entry.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[crprashant]

Good catch - I've moved the -- End of tests marker to the actual end of the file, after the Issue 2391 regression block, so it no longer implies the script terminates early. Fixed in c1a3cba.

[crprashant]

Agreed - map key order isn't guaranteed and isn't what this test asserts. I've dropped the misleading in order wording from the comment (it now reads mixed non-null and null values are all preserved). Fixed in c1a3cba.