gnucash stable: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Sun Jun 21 18:46:32 EDT 2026
Updated via https://github.com/Gnucash/gnucash/commit/32a6868f (commit)
via https://github.com/Gnucash/gnucash/commit/ea93d42f (commit)
from https://github.com/Gnucash/gnucash/commit/59f0de82 (commit)
commit 32a6868f521fdb9a901f702774a3652616abf6e7
Merge: 59f0de826b ea93d42f58
Author: John Ralls <jralls at ceridwen.us>
Date: Sun Jun 21 15:41:47 2026 -0700
Merge Noerr-Noah's 'bug-split-setparent-kvp-slots' into stable.
commit ea93d42f58f671a2ad683c2e718abf7b8914698f
Author: Noah R <Noerr at users.noreply.github.com>
Date: Wed Jun 17 15:21:10 2026 -0700
Bug 799777 - xaccSplitSetParent: keep a reparented split's KVP slots
When an already-committed split is moved to another transaction with
xaccSplitSetParent(), the split was marked dirty *after* the old
transaction was committed. trans_cleanup_commit() only drops a
moved-out split from the old transaction's split list when the split is
dirty at that commit, so the still-clean split was left dangling in the
old transaction's split list.
On the SQL/DBI backends, destroying the now-vacated old transaction then
deleted the moved split's KVP slots (online_id, cap-gains links, ...) by
the split's own GUID, even though the split now belongs to another
transaction. The split's native columns survive -- they are rewritten
under the new tx_guid -- but its slots are lost; in some sequences the
save fails outright with "UNIQUE constraint failed: transactions.guid".
The XML backend is unaffected because it re-serializes the whole book.
Mark the split dirty before committing the old transaction so
trans_cleanup_commit() removes it from the old transaction's split list,
as that code already intends. This is a no-op for split creation (the
overwhelmingly common caller), where the prior parent is NULL.
Add a DBI round-trip regression test (sqlite3, and mysql/pgsql when the
TEST_*_URL variables are set): seed a split carrying an online_id, save
and reload, reparent it onto another transaction, destroy the old
transaction, then save and reload again and assert the online_id
survived.
diff --git a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
index 8ba7ce591e..00a6682542 100644
--- a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
+++ b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
@@ -438,6 +438,124 @@ test_dbi_store_and_reload (Fixture* fixture, gconstpointer pData)
qof_session_destroy (session_3);
}
+static void
+test_dbi_reparent_split_keeps_slots (Fixture* fixture, gconstpointer pData)
+{
+ const gchar* url = (const gchar*)pData;
+ const char* online_id = "reparent-online-id-1";
+ auto msg = "[GncDbiSqlConnection::unlock_database()] There was no lock entry in the Lock table";
+ auto loglevel = static_cast<GLogLevelFlags> (G_LOG_LEVEL_WARNING |
+ G_LOG_FLAG_FATAL);
+ TestErrorStruct* check = test_error_struct_new (nullptr, loglevel, msg);
+ fixture->hdlrs = test_log_set_fatal_handler (fixture->hdlrs, check,
+ (GLogFunc)test_checked_handler);
+ if (fixture->filename)
+ url = fixture->filename;
+
+ GncGUID moved_guid, src_guid, dst_guid;
+
+ /* ---- Tag a fixture split with an online_id and add a second txn. ---- */
+ {
+ auto book = qof_session_get_book (fixture->session);
+ auto bank = gnc_account_lookup_by_name (gnc_book_get_root_account (book),
+ "Bank 1");
+ g_assert_nonnull (bank);
+
+ /* Grab the transaction the fixture created. */
+ Transaction* src_txn = nullptr;
+ qof_collection_foreach (qof_book_get_collection (book, GNC_ID_TRANS),
+ [] (QofInstance* inst, gpointer data) {
+ auto found = static_cast<Transaction**> (data);
+ if (*found == nullptr)
+ *found = GNC_TRANSACTION (inst);
+ }, &src_txn);
+ g_assert_nonnull (src_txn);
+ auto currency = xaccTransGetCurrency (src_txn);
+ auto moved_split = xaccTransGetSplit (src_txn, 0);
+ g_assert_nonnull (moved_split);
+
+ xaccTransBeginEdit (src_txn);
+ xaccSplitSetAccount (moved_split, bank);
+ qof_instance_get_slots (QOF_INSTANCE (moved_split))->set (
+ {"online_id"}, new KvpValue (g_strdup (online_id)));
+ xaccTransCommitEdit (src_txn);
+
+ auto dst_txn = xaccMallocTransaction (book);
+ xaccTransBeginEdit (dst_txn);
+ xaccTransSetCurrency (dst_txn, currency);
+ auto dst_split = xaccMallocSplit (book);
+ xaccSplitSetParent (dst_split, dst_txn);
+ xaccSplitSetAccount (dst_split, bank);
+ xaccTransCommitEdit (dst_txn);
+
+ moved_guid = *qof_instance_get_guid (QOF_INSTANCE (moved_split));
+ src_guid = *qof_instance_get_guid (QOF_INSTANCE (src_txn));
+ dst_guid = *qof_instance_get_guid (QOF_INSTANCE (dst_txn));
+ }
+
+ /* ---- Save the fixture's book to the store. ---- */
+ auto save_book = qof_book_new ();
+ auto session_save = qof_session_new (save_book);
+ qof_session_begin (session_save, url, SESSION_NEW_OVERWRITE);
+ g_assert_cmpint (qof_session_get_error (session_save), ==, ERR_BACKEND_NO_ERR);
+ qof_session_swap_data (fixture->session, session_save);
+ qof_book_mark_session_dirty (qof_session_get_book (session_save));
+ qof_session_save (session_save, NULL);
+ g_assert_cmpint (qof_session_get_error (session_save), ==, ERR_BACKEND_NO_ERR);
+ qof_session_end (session_save);
+ qof_session_destroy (session_save);
+
+ /* ---- Reload, reparent the split, destroy the old transaction, save. ---- */
+ auto edit_book = qof_book_new ();
+ auto session_edit = qof_session_new (edit_book);
+ qof_session_begin (session_edit, url, SESSION_NORMAL_OPEN);
+ g_assert_cmpint (qof_session_get_error (session_edit), ==, ERR_BACKEND_NO_ERR);
+ qof_session_load (session_edit, NULL);
+ g_assert_cmpint (qof_session_get_error (session_edit), ==, ERR_BACKEND_NO_ERR);
+ {
+ auto book = qof_session_get_book (session_edit);
+ auto moved = xaccSplitLookup (&moved_guid, book);
+ auto src_txn = xaccTransLookup (&src_guid, book);
+ auto dst_txn = xaccTransLookup (&dst_guid, book);
+ g_assert_nonnull (moved);
+ g_assert_nonnull (src_txn);
+ g_assert_nonnull (dst_txn);
+ /* Sanity: the slot is present in the loaded book. */
+ g_assert_nonnull (qof_instance_get_slots (QOF_INSTANCE (moved))->get_slot (
+ {"online_id"}));
+
+ xaccSplitSetParent (moved, dst_txn); /* move from src_txn to dst_txn */
+ xaccTransBeginEdit (src_txn);
+ xaccTransDestroy (src_txn); /* destroy the vacated old txn */
+ xaccTransCommitEdit (src_txn);
+ }
+ qof_session_save (session_edit, NULL);
+ g_assert_cmpint (qof_session_get_error (session_edit), ==, ERR_BACKEND_NO_ERR);
+ qof_session_end (session_edit);
+ qof_session_destroy (session_edit);
+
+ /* ---- Reload again and verify the slot followed the split. ---- */
+ auto verify_book = qof_book_new ();
+ auto session_verify = qof_session_new (verify_book);
+ qof_session_begin (session_verify, url, SESSION_READ_ONLY);
+ g_assert_cmpint (qof_session_get_error (session_verify), ==, ERR_BACKEND_NO_ERR);
+ qof_session_load (session_verify, NULL);
+ g_assert_cmpint (qof_session_get_error (session_verify), ==, ERR_BACKEND_NO_ERR);
+ {
+ auto book = qof_session_get_book (session_verify);
+ auto moved = xaccSplitLookup (&moved_guid, book);
+ g_assert_nonnull (moved); /* split survived */
+ g_assert_true (xaccSplitGetParent (moved) ==
+ xaccTransLookup (&dst_guid, book)); /* moved to dst_txn */
+ auto slot = qof_instance_get_slots (QOF_INSTANCE (moved))->get_slot (
+ {"online_id"});
+ g_assert_nonnull (slot); /* slot retained through the reparent */
+ g_assert_cmpstr (slot->get<const char*> (), ==, online_id);
+ }
+ qof_session_end (session_verify);
+ qof_session_destroy (session_verify);
+}
+
/** Test the safe_save mechanism. Beware that this test used on its
* own doesn't ensure that the resave is done safely, only that the
* database is intact and unchanged after the save. To observe the
@@ -659,6 +777,8 @@ create_dbi_test_suite (const char* dbm_name, const char* url)
auto subsuite = g_strdup_printf ("%s/%s", suitename, dbm_name);
GNC_TEST_ADD (subsuite, "store_and_reload", Fixture, url, setup,
test_dbi_store_and_reload, teardown);
+ GNC_TEST_ADD (subsuite, "reparent_split_keeps_slots", Fixture, url,
+ setup_memory, test_dbi_reparent_split_keeps_slots, teardown);
GNC_TEST_ADD (subsuite, "safe_save", Fixture, url, setup_memory,
test_dbi_safe_save, teardown);
GNC_TEST_ADD (subsuite, "version_control", Fixture, url, setup_memory,
diff --git a/libgnucash/engine/Split.cpp b/libgnucash/engine/Split.cpp
index ad257808b4..2d0c3ecb41 100644
--- a/libgnucash/engine/Split.cpp
+++ b/libgnucash/engine/Split.cpp
@@ -1860,9 +1860,9 @@ xaccSplitSetParent(Split *s, Transaction *t)
qof_event_gen(&old_trans->inst, GNC_EVENT_ITEM_REMOVED, &ed);
}
s->parent = t;
+ qof_instance_set_dirty(QOF_INSTANCE(s));
xaccTransCommitEdit(old_trans);
- qof_instance_set_dirty(QOF_INSTANCE(s));
if (t)
{
Summary of changes:
.../backend/dbi/test/test-backend-dbi-basic.cpp | 120 +++++++++++++++++++++
libgnucash/engine/Split.cpp | 2 +-
2 files changed, 121 insertions(+), 1 deletion(-)
More information about the gnucash-changes
mailing list