After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 795944 - Cannot store change to Business Suppliers data
Cannot store change to Business Suppliers data
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Backend - SQL
3.1
Other Mac OS
: Normal normal
: future
Assigned To: gnucash-core-maint
gnucash-core-maint
Depends on:
Blocks:
 
 
Reported: 2018-05-08 18:16 UTC by Jo Wetzig
Modified: 2018-06-30 00:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Crash report on opening GC after changing Business Suppliers Data in an SQL GC data file (70.14 KB, text/plain)
2018-05-08 18:16 UTC, Jo Wetzig
Details

Description Jo Wetzig 2018-05-08 18:16:33 UTC
Created attachment 371814 [details]
Crash report on opening GC after changing Business Suppliers Data in an SQL GC data file

Running (localised German) GnuCash Version: 3.1, Build ID: 3.0-118-gd2ef5fd0f+ (2018-04-28) with
Finance::Quote: 1.47 on a MacBook Pro Mid 2012 with Sierra 10.12.6
Data is stored in an sql file.

Please note: all English message texts below are my re-translation from German and may read differently in the original GC.

Problem:
Opening the Business>Suppliers>Suppliers Overwiew Menu and then attempting to modify an existing Business Suppliers Address after double-clicking on it and changing some entry/ies upon clicking OK the change pops up a warning
  "Datenbank konnte nicht gespeichert werden" (Database could not be stored)
The displayed suppliers overview table DOES show the alteration, however.
Trying to close GC then brings up an error alert:
   "Der Server unter der URL <my_path_without_any_special_characters_or_blanks> stiess auf einen Fehler oder benutzte falsche oder fehlerhafte Daten." (The server at the URL ... encontered an error or used wrong or faulty data) and GC offers to save the data in a Save Dialog. Saving the data closes GC apparently in a regular manner, but at the next start it crashes with a segmentation fault 11 when trying to open the saved file. 
The crash report is appended.
The saved file is markedly smaller than the last good version; in my case 23.8 MB vs. 43.3 MB.

Workaround:
open GC from the command line with the "--nofile" parameter and open the old unchanged good data file by the File>Open menu (unless you overwrote it in the offered Save dialog).

Seems to me the data change stores a local copy which it displays, but lies about not changing the data file and/or at the same time corrupts it.

BTW: changing the customers addresses works, even though the dialogs look much the. same.

Oh well, I'll have to keep my suppliers in order manually, for now....

Cheers

Joachim
Comment 1 Jo Wetzig 2018-05-09 04:27:11 UTC
Upon testing:

This is a regression from V 2.6.21 to V 3.0
V 2.6.21 (Version 868489b1c+ from 2018-04-10) can modify the suppliers data without displaying the warning or the error and leaves the sql file intact, V 3.0 cannot.

J W
Comment 2 John Ralls 2018-05-26 19:53:56 UTC
Your crash report shows the XML backend loading the file, title and component changed accordingly.
Comment 3 Geert Janssens 2018-06-23 11:24:58 UTC
The crash report is from trying to load the xml file that was created by trying to save the corrupted sqlite3 book to xml *after* the error occurred.

So it's only useful to be more robust in case we get in the situation where the sqlite3 db gets corrupted.

The original issue does start in the sql backend so the original component was correct.

I have figured out where it goes wrong and will detail the issue here for posterity.

The quick summary is that when opening an sql book vendors are loaded but their qof instances retain their infant state (that is is_infant remains TRUE).

When the code tries to commit a change later on (like a change in address as Jo describes here) the infant state makes the sql code assume the vendor does not exist in sql and so the code issues an INSERT statement instead of an UPDATE statement. Obviously this will fail as the object does exist in sql.

This can be observed by setting a breakpoint in
https://github.com/Gnucash/gnucash/blob/maint/libgnucash/backend/sql/gnc-vendor-sql.cpp#L152
after loading the file and then try to change a vendor to observe is_infant remains 1

This is not so for customer objects. They don't have their own commit specialization so you'll have to break in
https://github.com/Gnucash/gnucash/blob/maint/libgnucash/backend/sql/gnc-sql-object-backend.cpp#L44
after loading a file and then try to change a customer to observe the customer object's infant state is 0.

This difference starts during file loading.

When loading an sql book, all loaded objects eventually pass via set_parameter in https://github.com/Gnucash/gnucash/blob/maint/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp#L569
And depending on whether the sql object definition for this object has setter functions defined or not this will then delegate to either
https://github.com/Gnucash/gnucash/blob/maint/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp#L545 (if only a g_object property is known)
or
https://github.com/Gnucash/gnucash/blob/maint/libgnucash/backend/sql/gnc-sql-column-table-entry.hpp#L531 (if a setter was passed)

And here's the rub. Any sql object type that has no setter for *any* of it's properties will stay in infant mode because the way the GObject based function was written: it used qof_instance_increase_edit_level/qof_instance_decrease_edit_level around the call to g_object_set. That prevents qof_commit_edit_part2 to run for the object:
1. It increases the edit level for the object
2. then calls g_object_set which may in turn call functions that have their own edit level management.
3. However due to the increased edit level this internal edit level management is disabled until we return to set_parameter
4. set_parameter then decreases the edit level again. In most cases this makes the edit level 0 again.
5. However most of our code will check for this and then run qof_commit_edit_part2 to complete the commit (and drop the infant state for the object)

A GncSqlCustomerBackend object has setters defined for it's ID so it will use the variant of set_parameter using a setter. That function does not manipulate the edit_level in the same way and the setter will eventually call qof_commit_edit_part2, resetting the infant state. So that works around this issue.
A GncSqlVendorBackend object has not so it's hit by this issue.

I have replaced the increase and decrease of edit level with proper calls to qof_begin_edit and qof_commit_edit{_part2} to force all objects loaded from sql to eventually reset their infant state.

Will be part of gnucash 3.2.
Comment 4 John Ralls 2018-06-30 00:09:37 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=795944. Please update any external references or bookmarks.