From 7aca73bc6f996a16a568cf33447e0cabe9770764 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Mon, 10 Aug 2026 10:46:23 -0700 Subject: [PATCH] fix: report successful zero-column Cypher statements as success, not error (#6) ladybug.cypher() is declared RETURNS SETOF record, so callers must supply a column definition list. Statements that legitimately return zero columns -- a data CREATE/MERGE/DELETE without RETURN, or a CALL of a void procedure -- execute successfully in Ladybug and then fail the column-count check in ladybug_bridge_execute_collect() (and the sibling ladybug_bridge_fill_tuplestore_from_query()): ERROR: ladybug: failed to execute cypher via Ladybug engine DETAIL: ladybug: column count mismatch: query returns 0 columns, expected 1 The side effect has already landed, so the caller sees an error for a write that in fact succeeded; an autocommit retry would double-apply it (or fail on a duplicate key, surfacing as yet another error). liblbug reports such statements as success=true with num_columns=0 and no rows-changed count exposed via the C API, so synthesize a result instead of failing: - When the caller supplied the conventional single-TEXT status shape ("AS t(ok text)"), return one row carrying "OK" so the caller can confirm the command completed. - Otherwise, return an honest empty result set (0 rows), not a false failure. Genuine column-count mismatches (non-zero, wrong count) still raise the same error as before. Add an issue #6 regression test that runs the full sequence in one backend against a dedicated store: CREATE NODE TABLE (DDL, status column -- already fine), data CREATE (0 columns -> "OK"), MERGE (0 columns, int column list -> count 0, not error), then MATCH confirming both writes landed. Fixes #6. --- ladybug_bridge.c | 76 ++++++++++++++++++++++++++++++++++++ scripts/test_with_pgembed.py | 44 ++++++++++++++++++++- 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/ladybug_bridge.c b/ladybug_bridge.c index f982c22..d70eb22 100644 --- a/ladybug_bridge.c +++ b/ladybug_bridge.c @@ -642,6 +642,43 @@ ladybug_bridge_fill_tuplestore_from_query(LadybugBridge *b, if (num_cols != natts) { + /* + * A successful statement that returns no result schema + * (num_cols == 0) -- e.g. a data CREATE/MERGE/DELETE without + * RETURN, or a CALL of a void procedure -- has already applied + * its side effect to the Ladybug engine. Raising an ERROR here + * would report a successful mutation as a failure and invite + * unsafe retries (see issue #6). Report success instead: + * + * - When the caller supplied a single TEXT column (the + * conventional "AS t(ok text)" status shape), synthesize one + * command-status row so the caller can confirm the command + * completed. + * - Otherwise, return zero rows: an honest empty result set, + * not a false failure. + */ + if (num_cols == 0) + { + lbug_query_result_destroy(&result); + + if (natts == 1 && + TupleDescAttr(tupdesc, 0)->atttypid == TEXTOID) + { + HeapTuple status_tuple; + Datum values[1]; + bool nulls[1]; + + values[0] = CStringGetTextDatum("OK"); + nulls[0] = false; + status_tuple = heap_form_tuple(tupdesc, values, nulls); + tuplestore_puttuple(ts, status_tuple); + heap_freetuple(status_tuple); + return 1; + } + + return 0; + } + if (err_msg) *err_msg = psprintf("ladybug: column count mismatch: query returns %d columns, expected %d", num_cols, natts); @@ -815,6 +852,45 @@ ladybug_bridge_execute_collect(LadybugBridge *b, if (num_cols != natts) { + /* + * A successful statement that returns no result schema + * (num_cols == 0) -- e.g. a data CREATE/MERGE/DELETE without + * RETURN, or a CALL of a void procedure -- has already applied + * its side effect to the Ladybug engine. Raising an ERROR here + * would report a successful mutation as a failure and invite + * unsafe retries (see issue #6). Report success instead: + * + * - When the caller supplied a single TEXT column (the + * conventional "AS t(ok text)" status shape), synthesize one + * command-status row so the caller can confirm the command + * completed. + * - Otherwise, return zero rows: an honest empty result set, + * not a false failure. + */ + if (num_cols == 0) + { + lbug_query_result_destroy(&result); + + if (natts == 1 && + TupleDescAttr(tupdesc, 0)->atttypid == TEXTOID) + { + HeapTuple status_tuple; + Datum values[1]; + bool nulls[1]; + + values[0] = CStringGetTextDatum("OK"); + nulls[0] = false; + status_tuple = heap_form_tuple(tupdesc, values, nulls); + + *out_tuples = (HeapTuple *) palloc(sizeof(HeapTuple)); + (*out_tuples)[0] = status_tuple; + return 1; + } + + *out_tuples = NULL; + return 0; + } + if (err_msg) *err_msg = psprintf("ladybug: column count mismatch: query returns %d columns, expected %d", num_cols, natts); diff --git a/scripts/test_with_pgembed.py b/scripts/test_with_pgembed.py index f0cd461..e5f911e 100644 --- a/scripts/test_with_pgembed.py +++ b/scripts/test_with_pgembed.py @@ -470,6 +470,48 @@ def check(o, e): "SELECT ladybug.disable_replication('repl2') AS n", env, check=lambda o, e: "1" in o) + # ================================================================ + # Issue #6 regression: a successful zero-column Cypher statement + # (e.g. a data CREATE / MERGE / DELETE without RETURN) executes + # successfully in Ladybug and then must NOT be reported to the + # caller as a column-count-mismatch error. Before the fix, + # ladybug_bridge_execute_collect rejected any result whose + # column count didn't match the caller's column definition + # list, so a statement that legitimately returns 0 columns + # failed at the column-count check -- after the side effect + # had already landed, inviting unsafe retries. + # + # The whole sequence runs in ONE backend against a dedicated + # storage path: create the native node table (DDL returns a + # status column, already fine), then a data CREATE (returns 0 + # columns -> synthesized "OK" status row for the conventional + # AS t(ok text) shape), then a MERGE (0 columns, but the + # caller's column list is int -> honest empty result, count 0, + # NOT an error), then a MATCH confirming both writes landed. + # All four statements share one psql -c so the store is + # created and reused in a single backend. + # ================================================================ + ISSUE6_STORE = "/tmp/pglb_issue6.lbdb" + run_test("Issue #6: zero-column Cypher mutations succeed (not reported as errors)", + f"SET ladybug.storage_path = '{ISSUE6_STORE}';" + f"SET ladybug.pg_connstr = '{libpq_connstr}';" + "SELECT * FROM ladybug.cypher(" # create native node table (DDL) + "$$CREATE NODE TABLE City(id INT64, name STRING, PRIMARY KEY(id))$$)" + " AS t(ok text);" + "SELECT * FROM ladybug.cypher(" # data CREATE -> 0 columns -> "OK" + "$$CREATE (n:City {id: 1, name: 'Toronto'})$$)" + " AS t(ok text);" + "SELECT count(*)::int AS cnt FROM ladybug.cypher(" # MERGE -> 0 columns, int col -> empty + "$$MERGE (n:City {id: 2, name: 'Montreal'})$$)" + " AS t(dummy int);" + "SELECT * FROM ladybug.cypher(" # confirm both writes landed + "$$MATCH (n:City) RETURN n.id, n.name ORDER BY n.id$$)" + " AS t(id bigint, name text)", + env, check=lambda o, e: ("OK" in o + and "Toronto" in o + and "Montreal" in o + and "ERROR" not in e.upper())) + xfail_note = f" ({tests_xfail} xfail)" if tests_xfail else "" print(f"\n=== {tests_passed}/{tests_total} tests passed{xfail_note} ===") # All existing tests are required. @@ -482,4 +524,4 @@ def check(o, e): if __name__ == "__main__": - sys.exit(main()) \ No newline at end of file + sys.exit(main())