Reverted some changes to _make_table function and added a _make_indices

function.
Moved Index into db.py from types.py

Author: semohr

beets/dbcore/__init__.py

@@ -16,7 +16,7 @@
 Library.
 """
 
-from .db import Database, Model, Results
+from .db import Database, Index, Model, Results
 from .query import (
     AndQuery,
     FieldQuery,
@@ -43,6 +43,7 @@
     "Query",
     "Results",
     "Type",
+    "Index",
     "parse_sorted_query",
     "query_from_strings",
     "sort_from_strings",

beets/dbcore/db.py

@@ -26,7 +26,7 @@
 from collections import defaultdict
 from collections.abc import Generator, Iterable, Iterator, Mapping, Sequence
 from sqlite3 import Connection
-from typing import TYPE_CHECKING, Any, AnyStr, Callable, Generic
+from typing import TYPE_CHECKING, Any, AnyStr, Callable, Generic, NamedTuple
 
 from typing_extensions import TypeVar  # default value support
 from unidecode import unidecode
@@ -287,7 +287,7 @@ class Model(ABC, Generic[D]):
     terms.
     """
 
-    _indices: Sequence[types.Index] = ()
+    _indices: Sequence[Index] = ()
     """A sequence of `Index` objects that describe the indices to be
     created for this table.
     """
@@ -1036,10 +1036,9 @@ def __init__(self, path, timeout: float = 5.0):
 
         # Set up database schema.
         for model_cls in self._models:
-            self._make_table(
-                model_cls._table, model_cls._fields, model_cls._indices
-            )
+            self._make_table(model_cls._table, model_cls._fields)
             self._make_attribute_table(model_cls._flex_table)
+            self._migrate_indices(model_cls._table, model_cls._indices)
 
     # Primitive access control: connections and transactions.
 
@@ -1160,32 +1159,20 @@ def load_extension(self, path: str):
 
     # Schema setup and migration.
 
-    def _make_table(
-        self,
-        table: str,
-        fields: Mapping[str, types.Type],
-        indices: Sequence[types.Index],
-    ):
+    def _make_table(self, table: str, fields: Mapping[str, types.Type]):
         """Set up the schema of the database. `fields` is a mapping
         from field names to `Type`s. Columns are added if necessary.
         """
-        # Get current schema and existing indexes
+        # Get current schema.
         with self.transaction() as tx:
             rows = tx.query("PRAGMA table_info(%s)" % table)
-            current_fields = {row[1] for row in rows}
-            index_rows = tx.query(f"PRAGMA index_list({table})")
-            current_indices = {row[1] for row in index_rows}
+        current_fields = {row[1] for row in rows}
 
-        # Skip table creation if the current schema matches the
-        # requested schema (and no indexes are missing).
         field_names = set(fields.keys())
-        index_names = {index.name for index in indices}
-        if current_fields.issuperset(
-            field_names
-        ) and current_indices.issuperset(index_names):
+        if current_fields.issuperset(field_names):
+            # Table exists and has all the required columns.
             return
 
-        # Table schema handling
         if not current_fields:
             # No table exists.
             columns = []
@@ -1208,17 +1195,6 @@ def _make_table(
         with self.transaction() as tx:
             tx.script(setup_sql)
 
-        # Index handling
-        with self.transaction() as tx:
-            for index in indices:
-                if index.name in current_indices:
-                    continue
-
-                columns_str = ", ".join(index.columns)
-                tx.script(
-                    f"CREATE INDEX {index.name} ON {table} ({columns_str})"
-                )
-
     def _make_attribute_table(self, flex_table: str):
         """Create a table and associated index for flexible attributes
         for the given entity (if they don't exist).
@@ -1237,6 +1213,33 @@ def _make_attribute_table(self, flex_table: str):
                 """.format(flex_table)
             )
 
+    def _migrate_indices(
+        self,
+        table: str,
+        indices: Sequence[Index],
+    ):
+        """Create or replace indices for the given table.
+
+        If the indices already exists and are up to date (i.e., the
+        index name and columns match), nothing is done. Otherwise, the
+        indices are created or replaced.
+        """
+        with self.transaction() as tx:
+            index_rows = tx.query(f"PRAGMA index_list({table})")
+            current_indices = {Index.from_db(tx, row[1]) for row in index_rows}
+
+        _indices = set(indices)
+
+        if current_indices.issuperset(_indices):
+            return
+
+        # May also include missing indices.
+        changed_indices = _indices - current_indices
+
+        with self.transaction() as tx:
+            for index in changed_indices:
+                index.recreate(tx, table)
+
     # Querying.
 
     def _fetch(
@@ -1306,3 +1309,42 @@ def _get(
         exist.
         """
         return self._fetch(model_cls, MatchQuery("id", id)).get()
+
+
+class Index(NamedTuple):
+    """A helper class to represent the index
+    information in the database schema.
+    """
+
+    name: str
+    columns: Sequence[str]
+
+    def recreate(self, tx: Transaction, table: str) -> None:
+        """Recreate the index in the database.
+
+        This is useful when the index has been changed and needs to be
+        updated.
+        """
+        tx.script(f"DROP INDEX IF EXISTS {self.name}")
+        self.create(tx, table)
+
+    def create(self, tx: Transaction, table: str) -> None:
+        """Create the index in the database."""
+        return tx.script(
+            f"CREATE INDEX {self.name} ON {table} ({', '.join(self.columns)})"
+        )
+
+    @classmethod
+    def from_db(cls, tx: Transaction, name: str) -> Index:
+        """Create an Index object from the database if it exists.
+
+        The name has to exists in the database! Otherwise, an
+        Error will be raised.
+        """
+        rows = tx.query(f"PRAGMA index_info({name})")
+        columns = [row[2] for row in rows]
+        return cls(name, columns)
+
+    def __hash__(self) -> int:
+        """Unique hash for the index based on its name and columns."""
+        return hash((self.name, tuple(self.columns)))

beets/dbcore/types.py

@@ -20,15 +20,7 @@
 import time
 import typing
 from abc import ABC
-from typing import (
-    TYPE_CHECKING,
-    Any,
-    Generic,
-    NamedTuple,
-    Sequence,
-    TypeVar,
-    cast,
-)
+from typing import TYPE_CHECKING, Any, Generic, TypeVar, cast
 
 import beets
 from beets import util
@@ -462,21 +454,6 @@ def parse(self, string):
                 return self.null
 
 
-class Index(NamedTuple):
-    """A database index.
-
-    A helper class to represent the index
-    information in the schema.
-    """
-
-    name: str
-    columns: Sequence[str]
-    table: str | None = None
-    """As a index is normally bound to a table
-    we may omit it here.
-    """
-
-
 # Shared instances of common types.
 DEFAULT = Default()
 INTEGER = Integer()

beets/library/models.py

@@ -717,7 +717,7 @@ class Item(LibModel):
         "added": types.DATE,
     }
     _indices = (
-        types.Index(
+        dbcore.Index(
             name="idx_item_album_id",
             columns=("album_id",),
         ),

docs/changelog.rst

@@ -57,8 +57,8 @@ Bug fixes:
   :bug:`5699`
 * :doc:`plugins/discogs`: Beets will no longer crash if a release has been
   deleted, and returns a 404.
-* Beets now creates an index for ``album_id`` on the ``items`` sqlite table.
-  This speeds up queries that filter items by their album significantly.
+* Beets now creates an index for the ``album_id`` field in the ``items`` table.
+  This significantly speeds up queries that filter items by their album.
   :bug:`5809`
 
 For packagers:

test/test_dbcore.py

@@ -23,6 +23,7 @@
 import pytest
 
 from beets import dbcore
+from beets.dbcore.db import Index
 from beets.library import LibModel
 from beets.test import _common
 from beets.util import cached_classproperty
@@ -58,6 +59,7 @@ class ModelFixture1(LibModel):
     _sorts = {
         "some_sort": SortFixture,
     }
+    _indices = (Index("field_one_index", ("field_one",)),)
 
     @cached_classproperty
     def _types(cls):
@@ -784,3 +786,48 @@ def test_no_results(self):
             self.db._fetch(ModelFixture1, dbcore.query.FalseQuery()).get()
             is None
         )
+
+
+class TestIndex:
+    @pytest.fixture(autouse=True)
+    def db(self):
+        """Set up an in-memory SQLite database."""
+        db = DatabaseFixture1(":memory:")
+        yield db
+        db._connection().close()
+
+    @pytest.fixture
+    def sample_index(self):
+        """Fixture for a sample Index object."""
+        return Index(name="sample_index", columns=["field_one"])
+
+    def test_index_creation(self, db, sample_index):
+        """Test creating an index and checking its existence."""
+        with db.transaction() as tx:
+            sample_index.create(tx, "test")
+            indexes = (
+                db._connection().execute("PRAGMA index_list(test)").fetchall()
+            )
+        assert any(sample_index.name in index for index in indexes)
+
+    def test_from_db(self, db, sample_index):
+        """Test retrieving an index from the database."""
+        with db.transaction() as tx:
+            sample_index.create(tx, "test")
+            retrieved = Index.from_db(tx, sample_index.name)
+            assert retrieved == sample_index
+
+    def test_index_hashing_and_set_behavior(self, sample_index):
+        """Test the hashing and set behavior of the Index class."""
+        index_set = {sample_index}
+        similar_index = Index(name="sample_index", columns=["field_one"])
+
+        assert similar_index in index_set  # Should recognize similar attributes
+
+        different_index = Index(name="other_index", columns=["other_field"])
+        index_set.add(different_index)
+
+        assert len(index_set) == 2  # Should recognize distinct index
+
+        index_set.discard(sample_index)
+        assert similar_index not in index_set  # Should remove similar index