[#7689] fixed indexes update collection error

CHANGELOG.md

@@ -9,6 +9,9 @@
 
 - Added error marker for each collection tab and fixed the styles of the raw errors tooltip.
 
+- Fixed indexes collection update error ([#7689](https://github.com/pocketbase/pocketbase/issues/7689)).
+    _The fix comes with a system migration that resaves all collection models to ensure that their indexes are normalized and available in the collection model (it will also include indexes created manually via the sqlite3 cli or other external tools)._
+
 - Updated `modernc.org/sqlite` to v1.50.1 (SQLite 3.53.1).
 
 - Other minor fixes (_updated API preview examples, fixed code comment typos, etc._).

core/collection_model.go

@@ -348,6 +348,7 @@ func (app *BaseApp) registerCollectionHooks() {
 }
 
 // @todo experiment eventually replacing the rules *string with a struct?
+// @todo consider changing the Indexes field to a "getter" for the sqlite_master table?
 type baseCollection struct {
 	BaseModel
 
@@ -820,6 +821,25 @@ func onCollectionSave(e *CollectionEvent) error {
 
 	e.Collection.updateGeneratedIdIfExists(e.App)
 
+	// normalize indexes table name
+	for i, raw := range e.Collection.Indexes {
+		parsed := dbutils.ParseIndex(raw)
+
+		// no need to normalize
+		if parsed.TableName == e.Collection.Name {
+			continue
+		}
+
+		parsed.TableName = e.Collection.Name
+
+		normalized := parsed.Build()
+		if normalized == "" {
+			continue // leave to the model validator to decide whether to return an error
+		}
+
+		e.Collection.Indexes[i] = normalized
+	}
+
 	return e.Next()
 }
 

core/collection_model_test.go

@@ -1678,3 +1678,40 @@ func TestCollectionSaveViewWrapping(t *testing.T) {
 		})
 	}
 }
+
+func TestCollectionSaveIndexesTableNameNormalization(t *testing.T) {
+	t.Parallel()
+
+	app, _ := tests.NewTestApp()
+	defer app.Cleanup()
+
+	dummyCollection := core.NewBaseCollection("new_test")
+	dummyCollection.Fields.Add(&core.TextField{Name: "test"})
+	dummyCollection.Indexes = []string{
+		"create index `new_test_idx1` on `` (`test`) where 1=1",
+		"create index `new_test_idx2` on `test` (`test`) where 1=2",
+		"create index `new_test_idx3` on `someting_else` (`test`) where 1=3",
+	}
+
+	err := app.Save(dummyCollection)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// refetch a clean state
+	dummyCollection, err = app.FindCollectionByNameOrId(dummyCollection.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(dummyCollection.Indexes) != 3 {
+		t.Fatalf("Expected 3 indexes, got %v", dummyCollection.Indexes)
+	}
+
+	for _, raw := range dummyCollection.Indexes {
+		parsed := dbutils.ParseIndex(raw)
+		if parsed.TableName != dummyCollection.Name {
+			t.Fatalf("Expected all indexes to have tableName %q, found %q:\n%s", dummyCollection.Name, parsed.TableName, raw)
+		}
+	}
+}

core/collection_record_table_sync.go

@@ -306,8 +306,10 @@ func dropCollectionIndexes(app App, collection *Collection) error {
 		for _, raw := range collection.Indexes {
 			parsed := dbutils.ParseIndex(raw)
 
-			if !parsed.IsValid() {
+			// note: don't check IsValid because the index table name may not be populated
-				continue
+			// (https://github.com/pocketbase/pocketbase/issues/7689)
+			if parsed.IndexName == "" {
+				return fmt.Errorf("failed to dop index - missing index name: %s", raw)
 			}
 
 			_, err := txApp.DB().NewQuery(fmt.Sprintf("DROP INDEX IF EXISTS [[%s]]", parsed.IndexName)).Execute()

core/collection_record_table_sync_test.go

@@ -294,3 +294,49 @@ func TestSingleVsMultipleValuesNormalization(t *testing.T) {
 		})
 	}
 }
+
+func TestDropIndexWithoutTableName(t *testing.T) {
+	t.Parallel()
+
+	app, _ := tests.NewTestApp()
+	defer app.Cleanup()
+
+	properIndex := "CREATE INDEX `new_test_idx2` ON `new_test` (`test`)"
+	indexWithoutTableName := "CREATE INDEX `new_test_idx2` ON `` (`test`)"
+
+	dummyCollection := core.NewBaseCollection("new_test")
+	dummyCollection.Fields.Add(&core.TextField{Name: "test"})
+	dummyCollection.Indexes = []string{properIndex}
+
+	err := app.Save(dummyCollection)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// resave without table name but without hooks to avoid the normalizations
+	dummyCollection.Indexes[0] = indexWithoutTableName
+	err = app.UnsafeWithoutHooks().Save(dummyCollection)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dummyCollection, err = app.FindCollectionByNameOrId(dummyCollection.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// resave should normalize the index
+	err = app.Save(dummyCollection)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dummyCollection, err = app.FindCollectionByNameOrId(dummyCollection.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(dummyCollection.Indexes) != 1 || dummyCollection.Indexes[0] != properIndex {
+		t.Fatalf("Expected exactly 1 index\n%s\ngot\n%v", properIndex, dummyCollection.Indexes)
+	}
+}

migrations/1778828400_normalize_indexes.go

@@ -0,0 +1,115 @@
+package migrations
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/pocketbase/dbx"
+	"github.com/pocketbase/pocketbase/core"
+	"github.com/pocketbase/pocketbase/tools/dbutils"
+)
+
+// see https://github.com/pocketbase/pocketbase/issues/7689
+func init() {
+	core.SystemMigrations.Register(func(txApp core.App) error {
+		collections, err := txApp.FindAllCollections()
+		if err != nil {
+			return err
+		}
+
+		for _, collection := range collections {
+			// existing system collection indexes can't be modified and view don't have indexes
+			if collection.System || collection.IsView() {
+				continue
+			}
+
+			masterIndexes := []struct {
+				Name string `db:"name"`
+				SQL  string `db:"sql"`
+			}{}
+
+			err := txApp.DB().Select("name", "sql").
+				From("sqlite_master").
+				AndWhere(dbx.HashExp{
+					"type":     "index",
+					"tbl_name": collection.Name,
+				}).
+				AndWhere(dbx.NewExp("sql IS NOT NULL AND name NOT LIKE 'sqlite_autoindex_%'")).
+				All(&masterIndexes)
+			if err != nil {
+				return err
+			}
+
+			// no indexes
+			if len(masterIndexes) == 0 && len(collection.Indexes) == 0 {
+				continue
+			}
+
+			missingParsedIndexes := map[string]dbutils.Index{}
+
+			// find missing master indexes
+		masterLoop:
+			for _, masterIndex := range masterIndexes {
+				mParsed := dbutils.ParseIndex(masterIndex.SQL)
+				mParsed.SchemaName = ""
+				mParsed.TableName = collection.Name
+
+				for _, raw := range collection.Indexes {
+					cParsed := dbutils.ParseIndex(raw)
+
+					// index already exists (if needed it will be normalized on resave)
+					if strings.EqualFold(cParsed.IndexName, mParsed.IndexName) {
+						continue masterLoop
+					}
+				}
+
+				missingParsedIndexes[masterIndex.Name] = mParsed
+			}
+
+		missingIndexesLoop:
+			for _, missing := range missingParsedIndexes {
+				missingSQL := missing.Build()
+
+				// drop the missing index to recreate later
+				_, err := txApp.DB().DropIndex(missing.TableName, missing.IndexName).Execute()
+				if err != nil {
+					return fmt.Errorf("failed to drop index %s: %w", missing.IndexName, err)
+				}
+
+				// no recreate: duplicated single unique tokenKey or email
+				// (auth collections are guaranteed to have them)
+				if collection.IsAuth() && missing.Unique && len(missing.Columns) == 1 &&
+					(strings.EqualFold(missing.Columns[0].Name, core.FieldNameTokenKey) || strings.EqualFold(missing.Columns[0].Name, core.FieldNameEmail)) {
+					continue missingIndexesLoop
+				}
+
+				// no recreate: the same index definition alreay exists
+				// in the collection but with different name
+				for _, raw := range collection.Indexes {
+					cParsed := dbutils.ParseIndex(raw)
+					cParsed.IndexName = missing.IndexName
+					cParsed.SchemaName = missing.SchemaName
+					cParsed.TableName = missing.TableName
+					cSQL := cParsed.Build()
+
+					if missingSQL == cSQL {
+						continue missingIndexesLoop
+					}
+				}
+
+				// recreate: add the missing index to the collection list and
+				// leave the user to decide whether they want to keep it or not
+				// (the index could have been previously created externally, e.g. via the sqlite3 cli)
+				collection.Indexes = append(collection.Indexes, missingSQL)
+			}
+
+			// resave to trigger indexes normalization
+			err = txApp.Save(collection)
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	}, nil)
+}