Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add blob columns for bsddb upgrade #1969

Open
wants to merge 3 commits into
base: maintenance/gramps60
Choose a base branch
from

Conversation

dsblank
Copy link
Member

@dsblank dsblank commented Feb 13, 2025

This PR fixes an oversight for bsddb upgrades.

@Nick-Hall Nick-Hall added the bug label Feb 13, 2025
@Nick-Hall Nick-Hall added this to the v6.0 milestone Feb 13, 2025
@Nick-Hall Nick-Hall changed the title Add blob_columns for bsddb upgrade Add blob_data columns for bsddb upgrade Feb 14, 2025
@prculley
Copy link
Contributor

So I had time to try this out, and we are far from done. I had to change the metadata blob location to "value" rather than "blob_data". Then I had to enclose some db.execute with transaction begin/commit to avoid a crash there.

Here is the code I used:

+++ b/gramps/plugins/db/bsddb/bsddb.py
@@ -153,6 +153,10 @@ class DbBsddb(SQLite):
         self.set_total(total)
         # copy data from each dbmap to sqlite table
         for old_t, new_t, dbmap in table_list:
+            if new_t != "metadata":
+                self.dbapi.execute("ALTER TABLE %s ADD COLUMN blob_data BLOB;" % new_t)
+            else:
+                self.dbapi.execute("ALTER TABLE %s ADD COLUMN value BLOB;" % new_t)
             for key in dbmap.keys():
                 self.update()
                 # Try to unpickle data saved before db version 19:
@@ -183,6 +187,7 @@ class DbBsddb(SQLite):
                                 data[format_ix] = fmat
                     elif key == b"gender_stats":
                         # data is a dict, containing entries (see GenderStats)
+                        self._txn_begin()
                         self.dbapi.execute("DELETE FROM gender_stats")
                         g_sql = (
                             "INSERT INTO gender_stats "
@@ -192,6 +197,7 @@ class DbBsddb(SQLite):
                         for name in data:
                             female, male, unknown = data[name]
                             self.dbapi.execute(g_sql, [name, female, male, unknown])
+                        self._txn_commit()
                         continue  # don't need this in metadata anymore
                     elif key == b"default":
                         # convert to string and change key

The next issue is that the metadata created in this step is not suitable for later conversion from json, most of it is just "None" rather than textual json data. Here is what the db looks like after bsddb conversion (it crashes when reading metadata).
I suspect we need to make sure the entries are initialized with appropriate defaults.

image

@dsblank
Copy link
Member Author

dsblank commented Feb 14, 2025

@prculley added two fixes to this PR. Surely we must be getting close! Thanks for your patience.

@dsblank
Copy link
Member Author

dsblank commented Feb 14, 2025

Once @prculley verifies this PR works, I'll remove the adding of the value column in metadata, as we are going to add that on database creation.

@dsblank dsblank changed the title Add blob_data columns for bsddb upgrade Add blob columns for bsddb upgrade Feb 14, 2025
@dsblank dsblank self-assigned this Feb 14, 2025
@prculley
Copy link
Contributor

Next problem I had was that name_formats, when empty, should be initialized to a list.
For some reason some of the similar items (*_names, *_roles etc.) are getting initialized to an empty set??

                        for format_ix in range(len(data)):
                            fmat = data[format_ix]
                            if len(fmat) == 3:
                                fmat = fmat + (True,)
                                data[format_ix] = fmat
                        else:
                            data = []

The next problem appears after the bsddb conversion to sqlite is done. At this point, the sqlite db metadata contains both blob (value column) AND json_data.

When the dbloader loop opens the sqlite db again to continue the upgrade process, the code in gen.db.generic tests for blob/json

            self.set_serializer("json")
        else:
            self.set_serializer("blob")

which sets json since the metadata has both columns.
In order to complete the load, we need to be in blob mode, during the _get_metadata() that immediately follows, so we can proceed to the upgrade.
So we need a better way to test for the blob/json mode here...
I bypassed the test in my debugger, and the rest of the upgrade from version 17 seemed to work. I still want to do further tests, since my small test db is missing quite a few metadata types. And then to make sure that sqlite db upgrades still work right.
But enough for today.

@dsblank
Copy link
Member Author

dsblank commented Feb 17, 2025

I think we have these issues fixed in #1974

@Nick-Hall
Copy link
Member

I reverted #1974 because it caused a crash when exiting Gramps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants