From 5f632517e7c2b130747c5620d40dcbcf3b4737c6 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Tue, 26 Sep 2023 10:17:09 +0200 Subject: [PATCH 01/39] wip --- .../backends/postgresql/metadata/metadata.go | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 7ea34e63e558..54ab76ec7461 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -29,10 +29,13 @@ const ( ) // Collection represents collection metadata. +// +// Collection value should be immutable to avoid data races. +// Use [deepCopy] to replace the whole value instead of modifying fields of existing value. type Collection struct { Name string TableName string - // TODO https://github.com/FerretDB/FerretDB/issues/3375 + Settings Settings } // deepCopy returns a deep copy. @@ -44,6 +47,7 @@ func (c *Collection) deepCopy() *Collection { return &Collection{ Name: c.Name, TableName: c.TableName, + Settings: c.Settings.deepCopy(), } } @@ -62,3 +66,25 @@ func (c *Collection) Unmarshal(doc *types.Document) error { return nil } + +// Settings represents collection settings. +type Settings struct { + Indexes []IndexInfo `json:"indexes"` +} + +// deepCopy returns a deep copy. +func (s Settings) deepCopy() Settings { + indexes := make([]IndexInfo, len(s.Indexes)) + + for i, index := range s.Indexes { + indexes[i] = IndexInfo{ + Name: index.Name, + Key: slices.Clone(index.Key), + Unique: index.Unique, + } + } + + return Settings{ + Indexes: indexes, + } +} From 4632b385888d0f33a8fffdf0d90fab1ec87850d7 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Tue, 26 Sep 2023 10:25:42 +0200 Subject: [PATCH 02/39] wip --- .../backends/postgresql/metadata/metadata.go | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 54ab76ec7461..2e2f7a0ce40f 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -16,6 +16,8 @@ package metadata import ( + "golang.org/x/exp/slices" + "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/must" ) @@ -56,6 +58,7 @@ func (c *Collection) Marshal() *types.Document { return must.NotFail(types.NewDocument( "_id", c.Name, "table", c.TableName, + "settings", c.Settings.Marshal(), )) } @@ -64,6 +67,10 @@ func (c *Collection) Unmarshal(doc *types.Document) error { c.Name = must.NotFail(doc.Get("_id")).(string) c.TableName = must.NotFail(doc.Get("table")).(string) + var settings Settings + must.NoError(settings.Unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) + c.Settings = settings + return nil } @@ -88,3 +95,50 @@ func (s Settings) deepCopy() Settings { Indexes: indexes, } } + +// Marshal returns [*types.Document] for settings. +func (s Settings) Marshal() *types.Document { + indexes := make([]*types.Document, len(s.Indexes)) + + for i, index := range s.Indexes { + indexes[i] = must.NotFail(types.NewDocument( + "name", index.Name, + "key", index.Key, + "unique", index.Unique, + )) + } + + return must.NotFail(types.NewDocument( + "indexes", indexes, + )) +} + +// Unmarshal sets settings from [*types.Document]. +func (s *Settings) Unmarshal(doc *types.Document) error { + indexes := must.NotFail(doc.Get("indexes")).([]*types.Document) + + s.Indexes = make([]IndexInfo, len(indexes)) + + for i, index := range indexes { + s.Indexes[i] = IndexInfo{ + Name: must.NotFail(index.Get("name")).(string), + Key: must.NotFail(index.Get("key")).([]IndexKeyPair), + Unique: must.NotFail(index.Get("unique")).(bool), + } + } + + return nil +} + +// IndexInfo represents information about a single index. +type IndexInfo struct { + Name string `json:"name"` + Key []IndexKeyPair `json:"key"` + Unique bool `json:"unique"` +} + +// IndexKeyPair consists of a field name and a sort order that are part of the index. +type IndexKeyPair struct { + Field string `json:"field"` + Descending bool `json:"descending"` +} From 9547f3a396dba8eadca188fc2541ebd9f0c85dce Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Tue, 26 Sep 2023 20:54:28 +0200 Subject: [PATCH 03/39] wip --- .../backends/postgresql/metadata/metadata.go | 57 +++-- .../backends/postgresql/metadata/registry.go | 198 ++++++++++++++++++ .../postgresql/metadata/registry_test.go | 126 +++++++++++ 3 files changed, 366 insertions(+), 15 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 2e2f7a0ce40f..ec4895f2a085 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -16,8 +16,13 @@ package metadata import ( + "errors" + "golang.org/x/exp/slices" + "github.com/FerretDB/FerretDB/internal/util/iterator" + "github.com/FerretDB/FerretDB/internal/util/lazyerrors" + "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/must" ) @@ -67,9 +72,14 @@ func (c *Collection) Unmarshal(doc *types.Document) error { c.Name = must.NotFail(doc.Get("_id")).(string) c.TableName = must.NotFail(doc.Get("table")).(string) - var settings Settings - must.NoError(settings.Unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) - c.Settings = settings + if doc.Has("settings") { + var settings Settings + must.NoError(settings.Unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) + c.Settings = settings + } else { + // If settings are not present, we initialize them with empty indexes to avoid potential nil pointers. + c.Settings = Settings{Indexes: []IndexInfo{}} + } return nil } @@ -98,14 +108,15 @@ func (s Settings) deepCopy() Settings { // Marshal returns [*types.Document] for settings. func (s Settings) Marshal() *types.Document { - indexes := make([]*types.Document, len(s.Indexes)) + indexes := types.MakeArray(len(s.Indexes)) for i, index := range s.Indexes { - indexes[i] = must.NotFail(types.NewDocument( + must.NoError(indexes.Set(i, must.NotFail(types.NewDocument( "name", index.Name, + "dbindex", index.DBIndex, "key", index.Key, "unique", index.Unique, - )) + )))) } return must.NotFail(types.NewDocument( @@ -115,15 +126,30 @@ func (s Settings) Marshal() *types.Document { // Unmarshal sets settings from [*types.Document]. func (s *Settings) Unmarshal(doc *types.Document) error { - indexes := must.NotFail(doc.Get("indexes")).([]*types.Document) + indexes := must.NotFail(doc.Get("indexes")).(*types.Array) + + s.Indexes = make([]IndexInfo, indexes.Len()) + + iter := indexes.Iterator() + defer iter.Close() + + for { + i, v, err := iter.Next() + if errors.Is(err, iterator.ErrIteratorDone) { + break + } + + if err != nil { + return lazyerrors.Error(err) + } - s.Indexes = make([]IndexInfo, len(indexes)) + doc := v.(*types.Document) - for i, index := range indexes { s.Indexes[i] = IndexInfo{ - Name: must.NotFail(index.Get("name")).(string), - Key: must.NotFail(index.Get("key")).([]IndexKeyPair), - Unique: must.NotFail(index.Get("unique")).(bool), + Name: must.NotFail(doc.Get("name")).(string), + DBIndex: must.NotFail(doc.Get("dbindex")).(string), + Key: must.NotFail(doc.Get("key")).([]IndexKeyPair), + Unique: must.NotFail(doc.Get("unique")).(bool), } } @@ -132,9 +158,10 @@ func (s *Settings) Unmarshal(doc *types.Document) error { // IndexInfo represents information about a single index. type IndexInfo struct { - Name string `json:"name"` - Key []IndexKeyPair `json:"key"` - Unique bool `json:"unique"` + Name string `json:"name"` + DBIndex string `json:"dbindex"` // how the index is created in the DB, like TableName for Collection + Key []IndexKeyPair `json:"key"` + Unique bool `json:"unique"` } // IndexKeyPair consists of a field name and a sort order that are part of the index. diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 7a32cdbddf59..23c1b3465cd3 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -687,6 +687,204 @@ func (r *Registry) CollectionRename(ctx context.Context, dbName, oldCollectionNa return true, nil } +// IndexesCreate creates indexes in the collection. +// +// Existing indexes with given names are ignored (TODO?). +func (r *Registry) IndexesCreate(ctx context.Context, dbName, collectionName string, indexes []IndexInfo) error { + defer observability.FuncCall(ctx)() + + p, err := r.getPool(ctx) + if err != nil { + return lazyerrors.Error(err) + } + + r.rw.Lock() + defer r.rw.Unlock() + + return r.indexesCreate(ctx, p, dbName, collectionName, indexes) +} + +// indexesCreate creates indexes in the collection. +// +// Existing indexes with given names are ignored (TODO?). +// +// It does not hold the lock. +func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, collectionName string, indexes []IndexInfo) error { + defer observability.FuncCall(ctx)() + + _, err := r.collectionCreate(ctx, p, dbName, collectionName) + if err != nil { + return lazyerrors.Error(err) + } + + db := r.colls[dbName] + if db == nil { + panic("database does not exist") + } + + c := r.collectionGet(dbName, collectionName) + if c == nil { + panic("collection does not exist") + } + + created := make([]string, 0, len(indexes)) + + for _, index := range indexes { + if slices.ContainsFunc(c.Settings.Indexes, func(i IndexInfo) bool { return index.Name == i.Name }) { + continue + } + + h := fnv.New32a() + must.NotFail(h.Write([]byte(index.Name))) + s := h.Sum32() + dbIndex := fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) + index.DBIndex = dbIndex + + q := "CREATE " + + if index.Unique { + q += "UNIQUE " + } + + q += "INDEX %q ON %q (%s)" + + columns := make([]string, len(index.Key)) + for i, key := range index.Key { + + // if the field is nested (e.g. foo.bar), it needs to be translated to the correct json path (foo -> bar) + fs := strings.Split(key.Field, ".") + transformedParts := make([]string, len(fs)) + + for j, f := range fs { + // It's important to sanitize field.Field data here, as it's a user-provided value. + transformedParts[j] = quoteString(f) + } + + columns[i] = fmt.Sprintf("((%s->%s))", DefaultColumn, strings.Join(transformedParts, " -> ")) + if key.Descending { + columns[i] += " DESC" + } + } + + q = fmt.Sprintf(q, pgx.Identifier{dbName, c.TableName}.Sanitize(), index.DBIndex, strings.Join(columns, ", ")) + + if _, err = p.Exec(ctx, q); err != nil { + _ = r.indexesDrop(ctx, p, dbName, collectionName, created) + return lazyerrors.Error(err) + } + + created = append(created, index.Name) + c.Settings.Indexes = append(c.Settings.Indexes, index) + } + + b, err := sjson.Marshal(c.Marshal()) + if err != nil { + return lazyerrors.Error(err) + } + + arg, err := sjson.MarshalSingleValue(collectionName) + if err != nil { + return lazyerrors.Error(err) + } + + q := fmt.Sprintf( + `UPDATE %s SET %s = $1 WHERE %s = $2`, + pgx.Identifier{dbName, metadataTableName}.Sanitize(), + DefaultColumn, + IDColumn, + ) + + if _, err := p.Exec(ctx, q, string(b), arg); err != nil { + return lazyerrors.Error(err) + } + + r.colls[dbName][collectionName] = c + + return nil +} + +// IndexesDrop drops provided indexes for the given collection. +func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName string, toDrop []string) error { + defer observability.FuncCall(ctx)() + + p, err := r.getPool(ctx) + if err != nil { + return lazyerrors.Error(err) + } + + r.rw.Lock() + defer r.rw.Unlock() + + return r.indexesDrop(ctx, p, dbName, collectionName, toDrop) +} + +// indexesDrop remove given connection's indexes. +// +// Non-existing indexes are ignored (TODO?). +// +// If database or collection does not exist, nil is returned (TODO?). +// +// It does not hold the lock. +func (r *Registry) indexesDrop(ctx context.Context, p *pgxpool.Pool, dbName, collectionName string, indexNames []string) error { + defer observability.FuncCall(ctx)() + + c := r.collectionGet(dbName, collectionName) + if c == nil { + return nil + } + + for _, name := range indexNames { + i := slices.IndexFunc(c.Settings.Indexes, func(i IndexInfo) bool { return name == i.Name }) + if i < 0 { + continue + } + + q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Settings.Indexes[i].DBIndex}.Sanitize()) + if _, err := p.Exec(ctx, q); err != nil { + return lazyerrors.Error(err) + } + + c.Settings.Indexes = slices.Delete(c.Settings.Indexes, i, i+1) + } + + b, err := sjson.Marshal(c.Marshal()) + if err != nil { + return lazyerrors.Error(err) + } + + arg, err := sjson.MarshalSingleValue(collectionName) + if err != nil { + return lazyerrors.Error(err) + } + + q := fmt.Sprintf( + `UPDATE %s SET %s = $1 WHERE %s = $2`, + pgx.Identifier{dbName, metadataTableName}.Sanitize(), + DefaultColumn, + IDColumn, + ) + + if _, err := p.Exec(ctx, q, string(b), arg); err != nil { + return lazyerrors.Error(err) + } + + r.colls[dbName][collectionName] = c + + return nil +} + +// quoteString returns a string that is safe to use in SQL queries. +// +// Deprecated: Warning! Avoid using this function unless there is no other way. +// Ideally, use a placeholder and pass the value as a parameter instead of calling this function. +// +// This approach is used in github.com/jackc/pgx/v4@v4.18.1/internal/sanitize/sanitize.go. +func quoteString(str string) string { + // We need "standard_conforming_strings=on" and "client_encoding=UTF8" (checked in checkConnection), + // otherwise we can't sanitize safely: https://github.com/jackc/pgx/issues/868#issuecomment-725544647 + return "'" + strings.ReplaceAll(str, "'", "''") + "'" +} + // Describe implements prometheus.Collector. func (r *Registry) Describe(ch chan<- *prometheus.Desc) { prometheus.DescribeByCollect(r, ch) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index a1171b86f11d..4f82ee753a52 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -414,3 +414,129 @@ func TestRenameCollection(t *testing.T) { require.Equal(t, expected, actual) }) } + +func TestIndexesCreateDrop(t *testing.T) { + t.Parallel() + + connInfo := conninfo.New() + ctx := conninfo.Ctx(testutil.Ctx(t), connInfo) + + r, db, dbName := createDatabase(t, ctx) + collectionName := testutil.CollectionName(t) + + t.Cleanup(func() { + _, err := r.DatabaseDrop(ctx, dbName) + require.NoError(t, err) + }) + + toCreate := []IndexInfo{ + { + Name: "index_non_unique", + Key: []IndexKeyPair{ + { + Field: "f1", + Descending: false, + }, + { + Field: "f2", + Descending: true, + }, + }, + }, + { + Name: "index_unique", + Key: []IndexKeyPair{ + { + Field: "foo", + Descending: false, + }, + }, + Unique: true, + }, + } + + err := r.IndexesCreate(ctx, dbName, collectionName, toCreate) + require.NoError(t, err) + + collection, err := r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + + t.Run("NonUniqueIndex", func(t *testing.T) { + indexName := collection.TableName + "_index_non_unique" + q := fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = '%s'", indexName) + + var row pgx.Row + row, err = db.Query(ctx, q) + require.NoError(t, err) + + var sql string + require.NoError(t, row.Scan(&sql)) + + expected := fmt.Sprintf( + `CREATE INDEX "%s" ON "%s" (_ferretdb_sjson->'$.f1', _ferretdb_sjson->'$.f2' DESC)`, + indexName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) + + t.Run("UniqueIndex", func(t *testing.T) { + indexName := collection.TableName + "_index_unique" + q := fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = '%s'", indexName) + row := db.QueryRow(ctx, q) + + var sql string + require.NoError(t, row.Scan(&sql)) + + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$.foo')`, + indexName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) + + t.Run("DefaultIndex", func(t *testing.T) { + indexName := collection.TableName + "__id_" + q := "SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?" + row := db.QueryRow(ctx, q, indexName) + + var sql string + require.NoError(t, row.Scan(&sql)) + + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$._id')`, + indexName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) + + t.Run("CheckSettingsAfterCreation", func(t *testing.T) { + err = r.initCollections(ctx, dbName, db) + require.NoError(t, err) + + collection, err = r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + require.Equal(t, 3, len(collection.Settings.Indexes)) + }) + + t.Run("DropIndexes", func(t *testing.T) { + toDrop := []string{"index_non_unique", "index_unique"} + err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) + require.NoError(t, err) + + q := "SELECT count(*) FROM sqlite_master WHERE type = 'index' AND tbl_name = ?" + row := db.QueryRow(ctx, q, collection.TableName) + + var count int + require.NoError(t, row.Scan(&count)) + require.Equal(t, 1, count) // only default index + }) + + t.Run("CheckSettingsAfterDrop", func(t *testing.T) { + err = r.initCollections(ctx, dbName, db) + require.NoError(t, err) + + collection, err = r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + require.Equal(t, 1, len(collection.Settings.Indexes)) + }) +} From ada44988202206c2cf8b95786eeb6f3e567ad4df Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Tue, 26 Sep 2023 21:07:10 +0200 Subject: [PATCH 04/39] wip --- .../backends/postgresql/metadata/metadata.go | 17 +++++++++++++---- .../backends/postgresql/metadata/registry.go | 4 ++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index ec4895f2a085..7a3861fa2043 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -110,13 +110,22 @@ func (s Settings) deepCopy() Settings { func (s Settings) Marshal() *types.Document { indexes := types.MakeArray(len(s.Indexes)) - for i, index := range s.Indexes { - must.NoError(indexes.Set(i, must.NotFail(types.NewDocument( + for _, index := range s.Indexes { + key := types.MakeArray(len(index.Key)) + + for _, pair := range index.Key { + key.Append(must.NotFail(types.NewDocument( + "field", pair.Field, + "descending", pair.Descending, + ))) + } + + indexes.Append(must.NotFail(types.NewDocument( "name", index.Name, "dbindex", index.DBIndex, - "key", index.Key, + "key", key, "unique", index.Unique, - )))) + ))) } return must.NotFail(types.NewDocument( diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 23c1b3465cd3..53189a57d41b 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -746,7 +746,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c q += "UNIQUE " } - q += "INDEX %q ON %q (%s)" + q += "INDEX %s ON %s (%s)" columns := make([]string, len(index.Key)) for i, key := range index.Key { @@ -766,7 +766,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c } } - q = fmt.Sprintf(q, pgx.Identifier{dbName, c.TableName}.Sanitize(), index.DBIndex, strings.Join(columns, ", ")) + q = fmt.Sprintf(q, pgx.Identifier{index.DBIndex}.Sanitize(), pgx.Identifier{dbName, c.TableName}.Sanitize(), strings.Join(columns, ", ")) if _, err = p.Exec(ctx, q); err != nil { _ = r.indexesDrop(ctx, p, dbName, collectionName, created) From d31b7ba9d4ff9e04c838a6a019f5b0c5f7626098 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 27 Sep 2023 10:26:58 +0200 Subject: [PATCH 05/39] wip --- .../postgresql/metadata/registry_test.go | 121 +++++++++--------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 4f82ee753a52..f03b66adb4d2 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -462,81 +462,82 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) t.Run("NonUniqueIndex", func(t *testing.T) { - indexName := collection.TableName + "_index_non_unique" - q := fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = '%s'", indexName) - - var row pgx.Row - row, err = db.Query(ctx, q) - require.NoError(t, err) - - var sql string - require.NoError(t, row.Scan(&sql)) - - expected := fmt.Sprintf( - `CREATE INDEX "%s" ON "%s" (_ferretdb_sjson->'$.f1', _ferretdb_sjson->'$.f2' DESC)`, - indexName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) - - t.Run("UniqueIndex", func(t *testing.T) { - indexName := collection.TableName + "_index_unique" - q := fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = '%s'", indexName) - row := db.QueryRow(ctx, q) + dbIndex := "index_non_unique_13064d07" var sql string - require.NoError(t, row.Scan(&sql)) + err = db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, dbIndex, + ).Scan(&sql) + require.NoError(t, err) expected := fmt.Sprintf( - `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$.foo')`, - indexName, collection.TableName, + `CREATE INDEX %s ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, + dbIndex, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) + /* + t.Run("UniqueIndex", func(t *testing.T) { + indexName := collection.TableName + "_index_unique" + q := fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = '%s'", indexName) + row := db.QueryRow(ctx, q) + + var sql string + require.NoError(t, row.Scan(&sql)) + + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$.foo')`, + indexName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) - t.Run("DefaultIndex", func(t *testing.T) { - indexName := collection.TableName + "__id_" - q := "SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?" - row := db.QueryRow(ctx, q, indexName) + t.Run("DefaultIndex", func(t *testing.T) { + indexName := collection.TableName + "__id_" + q := "SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?" + row := db.QueryRow(ctx, q, indexName) - var sql string - require.NoError(t, row.Scan(&sql)) + var sql string + require.NoError(t, row.Scan(&sql)) - expected := fmt.Sprintf( - `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$._id')`, - indexName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$._id')`, + indexName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) - t.Run("CheckSettingsAfterCreation", func(t *testing.T) { - err = r.initCollections(ctx, dbName, db) - require.NoError(t, err) + t.Run("CheckSettingsAfterCreation", func(t *testing.T) { + err = r.initCollections(ctx, dbName, db) + require.NoError(t, err) - collection, err = r.CollectionGet(ctx, dbName, collectionName) - require.NoError(t, err) - require.Equal(t, 3, len(collection.Settings.Indexes)) - }) + collection, err = r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + require.Equal(t, 3, len(collection.Settings.Indexes)) + }) - t.Run("DropIndexes", func(t *testing.T) { - toDrop := []string{"index_non_unique", "index_unique"} - err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) - require.NoError(t, err) + t.Run("DropIndexes", func(t *testing.T) { + toDrop := []string{"index_non_unique", "index_unique"} + err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) + require.NoError(t, err) - q := "SELECT count(*) FROM sqlite_master WHERE type = 'index' AND tbl_name = ?" - row := db.QueryRow(ctx, q, collection.TableName) + q := "SELECT count(*) FROM sqlite_master WHERE type = 'index' AND tbl_name = ?" + row := db.QueryRow(ctx, q, collection.TableName) - var count int - require.NoError(t, row.Scan(&count)) - require.Equal(t, 1, count) // only default index - }) + var count int + require.NoError(t, row.Scan(&count)) + require.Equal(t, 1, count) // only default index + }) - t.Run("CheckSettingsAfterDrop", func(t *testing.T) { - err = r.initCollections(ctx, dbName, db) - require.NoError(t, err) + t.Run("CheckSettingsAfterDrop", func(t *testing.T) { + err = r.initCollections(ctx, dbName, db) + require.NoError(t, err) - collection, err = r.CollectionGet(ctx, dbName, collectionName) - require.NoError(t, err) - require.Equal(t, 1, len(collection.Settings.Indexes)) - }) + collection, err = r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + require.Equal(t, 1, len(collection.Settings.Indexes)) + }) + */ } From 61bb540b0cd4f2992b7830c7a9d5b16ed99974fd Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 27 Sep 2023 12:21:21 +0200 Subject: [PATCH 06/39] wip --- .../backends/postgresql/metadata/metadata.go | 3 +- .../backends/postgresql/metadata/registry.go | 20 ++- .../postgresql/metadata/registry_test.go | 137 +++++++++++------- 3 files changed, 99 insertions(+), 61 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 7a3861fa2043..4c7afae0e383 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -20,10 +20,9 @@ import ( "golang.org/x/exp/slices" + "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/iterator" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" - - "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/must" ) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 53189a57d41b..5f4547a9dc17 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -498,12 +498,6 @@ func (r *Registry) collectionCreate(ctx context.Context, p *pgxpool.Pool, dbName return false, lazyerrors.Error(err) } - // create PG index for collection name - // TODO https://github.com/FerretDB/FerretDB/issues/3375 - - // create PG index for table name - // TODO https://github.com/FerretDB/FerretDB/issues/3375 - q = fmt.Sprintf( `INSERT INTO %s (%s) VALUES ($1)`, pgx.Identifier{dbName, metadataTableName}.Sanitize(), @@ -522,6 +516,20 @@ func (r *Registry) collectionCreate(ctx context.Context, p *pgxpool.Pool, dbName } r.colls[dbName][collectionName] = c + err = r.indexesCreate(ctx, p, dbName, collectionName, + []IndexInfo{ + { + Name: "_id_", + Key: []IndexKeyPair{{Field: "_id"}}, + Unique: true, + }, + }, + ) + if err != nil { + _, _ = r.collectionDrop(ctx, p, dbName, collectionName) + return false, lazyerrors.Error(err) + } + return true, nil } diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index f03b66adb4d2..53d8b734f330 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -453,6 +453,18 @@ func TestIndexesCreateDrop(t *testing.T) { }, Unique: true, }, + { + Name: "nested_fields", + Key: []IndexKeyPair{ + { + Field: "foo.bar", + }, + { + Field: "foo.baz", + Descending: true, + }, + }, + }, } err := r.IndexesCreate(ctx, dbName, collectionName, toCreate) @@ -463,7 +475,6 @@ func TestIndexesCreateDrop(t *testing.T) { t.Run("NonUniqueIndex", func(t *testing.T) { dbIndex := "index_non_unique_13064d07" - var sql string err = db.QueryRow( ctx, @@ -478,66 +489,86 @@ func TestIndexesCreateDrop(t *testing.T) { ) require.Equal(t, expected, sql) }) - /* - t.Run("UniqueIndex", func(t *testing.T) { - indexName := collection.TableName + "_index_unique" - q := fmt.Sprintf("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = '%s'", indexName) - row := db.QueryRow(ctx, q) - - var sql string - require.NoError(t, row.Scan(&sql)) - - expected := fmt.Sprintf( - `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$.foo')`, - indexName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) - t.Run("DefaultIndex", func(t *testing.T) { - indexName := collection.TableName + "__id_" - q := "SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?" - row := db.QueryRow(ctx, q, indexName) + t.Run("UniqueIndex", func(t *testing.T) { + dbIndex := "index_unique_d29d5863" + var sql string + err = db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, dbIndex, + ).Scan(&sql) + require.NoError(t, err) - var sql string - require.NoError(t, row.Scan(&sql)) + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, + dbIndex, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) - expected := fmt.Sprintf( - `CREATE UNIQUE INDEX "%s" ON "%s" (_ferretdb_sjson->'$._id')`, - indexName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) + t.Run("NestedFields", func(t *testing.T) { + dbIndex := "nested_fields_21fa0586" + var sql string + err = db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, dbIndex, + ).Scan(&sql) + require.NoError(t, err) - t.Run("CheckSettingsAfterCreation", func(t *testing.T) { - err = r.initCollections(ctx, dbName, db) - require.NoError(t, err) + expected := fmt.Sprintf( + `CREATE INDEX %s ON %q.%s USING btree ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, + dbIndex, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) - collection, err = r.CollectionGet(ctx, dbName, collectionName) - require.NoError(t, err) - require.Equal(t, 3, len(collection.Settings.Indexes)) - }) + t.Run("DefaultIndex", func(t *testing.T) { + dbIndex := "_id__67399184" + var sql string + err = db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, dbIndex, + ).Scan(&sql) + require.NoError(t, err) - t.Run("DropIndexes", func(t *testing.T) { - toDrop := []string{"index_non_unique", "index_unique"} - err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) - require.NoError(t, err) + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, + dbIndex, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) - q := "SELECT count(*) FROM sqlite_master WHERE type = 'index' AND tbl_name = ?" - row := db.QueryRow(ctx, q, collection.TableName) + t.Run("CheckSettingsAfterCreation", func(t *testing.T) { + err = r.initCollections(ctx, dbName, db) + require.NoError(t, err) - var count int - require.NoError(t, row.Scan(&count)) - require.Equal(t, 1, count) // only default index - }) + collection, err = r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + require.Equal(t, 3, len(collection.Settings.Indexes)) + }) - t.Run("CheckSettingsAfterDrop", func(t *testing.T) { - err = r.initCollections(ctx, dbName, db) - require.NoError(t, err) + t.Run("DropIndexes", func(t *testing.T) { + toDrop := []string{"index_non_unique", "index_unique"} + err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) + require.NoError(t, err) - collection, err = r.CollectionGet(ctx, dbName, collectionName) - require.NoError(t, err) - require.Equal(t, 1, len(collection.Settings.Indexes)) - }) - */ + q := "SELECT count(*) FROM sqlite_master WHERE type = 'index' AND tbl_name = ?" + row := db.QueryRow(ctx, q, collection.TableName) + + var count int + require.NoError(t, row.Scan(&count)) + require.Equal(t, 1, count) // only default index + }) + + t.Run("CheckSettingsAfterDrop", func(t *testing.T) { + err = r.initCollections(ctx, dbName, db) + require.NoError(t, err) + + collection, err = r.CollectionGet(ctx, dbName, collectionName) + require.NoError(t, err) + require.Equal(t, 1, len(collection.Settings.Indexes)) + }) } From 86e88c65516c4d46ba8e9f5c65f729b0df55b47c Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 27 Sep 2023 14:25:43 +0200 Subject: [PATCH 07/39] wip --- .../backends/postgresql/metadata/metadata.go | 18 +++++----- .../backends/postgresql/metadata/registry.go | 27 +++++++++++++-- .../postgresql/metadata/registry_test.go | 33 +++++++++++++++++++ 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 4c7afae0e383..725a6850f80e 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -121,7 +121,7 @@ func (s Settings) Marshal() *types.Document { indexes.Append(must.NotFail(types.NewDocument( "name", index.Name, - "dbindex", index.DBIndex, + "dbindex", index.TableIndexName, "key", key, "unique", index.Unique, ))) @@ -154,10 +154,10 @@ func (s *Settings) Unmarshal(doc *types.Document) error { doc := v.(*types.Document) s.Indexes[i] = IndexInfo{ - Name: must.NotFail(doc.Get("name")).(string), - DBIndex: must.NotFail(doc.Get("dbindex")).(string), - Key: must.NotFail(doc.Get("key")).([]IndexKeyPair), - Unique: must.NotFail(doc.Get("unique")).(bool), + Name: must.NotFail(doc.Get("name")).(string), + TableIndexName: must.NotFail(doc.Get("dbindex")).(string), + Key: must.NotFail(doc.Get("key")).([]IndexKeyPair), + Unique: must.NotFail(doc.Get("unique")).(bool), } } @@ -166,10 +166,10 @@ func (s *Settings) Unmarshal(doc *types.Document) error { // IndexInfo represents information about a single index. type IndexInfo struct { - Name string `json:"name"` - DBIndex string `json:"dbindex"` // how the index is created in the DB, like TableName for Collection - Key []IndexKeyPair `json:"key"` - Unique bool `json:"unique"` + Name string `json:"name"` + TableIndexName string `json:"pgindex"` // how the index is created in the DB, like TableName for Collection + Key []IndexKeyPair `json:"key"` + Unique bool `json:"unique"` } // IndexKeyPair consists of a field name and a sort order that are part of the index. diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 5f4547a9dc17..8250d25ecd7c 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -333,6 +333,27 @@ func (r *Registry) databaseGetOrCreate(ctx context.Context, p *pgxpool.Pool, dbN return nil, lazyerrors.Error(err) } + q = fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %s (((%s)))`, + pgx.Identifier{metadataTableName + "_id_idx"}.Sanitize(), + pgx.Identifier{dbName, metadataTableName}.Sanitize(), + IDColumn, + ) + if _, err = p.Exec(ctx, q); err != nil { + _, _ = r.databaseDrop(ctx, p, dbName) + return nil, lazyerrors.Error(err) + } + q = fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %s (((%s->'table')))`, + pgx.Identifier{metadataTableName + "_table_idx"}.Sanitize(), + pgx.Identifier{dbName, metadataTableName}.Sanitize(), + DefaultColumn, + ) + if _, err = p.Exec(ctx, q); err != nil { + _, _ = r.databaseDrop(ctx, p, dbName) + return nil, lazyerrors.Error(err) + } + r.colls[dbName] = map[string]*Collection{} return p, nil @@ -746,7 +767,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c must.NotFail(h.Write([]byte(index.Name))) s := h.Sum32() dbIndex := fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) - index.DBIndex = dbIndex + index.TableIndexName = dbIndex q := "CREATE " @@ -774,7 +795,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c } } - q = fmt.Sprintf(q, pgx.Identifier{index.DBIndex}.Sanitize(), pgx.Identifier{dbName, c.TableName}.Sanitize(), strings.Join(columns, ", ")) + q = fmt.Sprintf(q, pgx.Identifier{index.TableIndexName}.Sanitize(), pgx.Identifier{dbName, c.TableName}.Sanitize(), strings.Join(columns, ", ")) if _, err = p.Exec(ctx, q); err != nil { _ = r.indexesDrop(ctx, p, dbName, collectionName, created) @@ -847,7 +868,7 @@ func (r *Registry) indexesDrop(ctx context.Context, p *pgxpool.Pool, dbName, col continue } - q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Settings.Indexes[i].DBIndex}.Sanitize()) + q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Settings.Indexes[i].TableIndexName}.Sanitize()) if _, err := p.Exec(ctx, q); err != nil { return lazyerrors.Error(err) } diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 53d8b734f330..b82099d639ad 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -20,6 +20,8 @@ import ( "sync/atomic" "testing" + "github.com/stretchr/testify/assert" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/require" @@ -571,4 +573,35 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, len(collection.Settings.Indexes)) }) + + t.Run("MetadataIndexes", func(t *testing.T) { + t.Parallel() + + var sql string + err = db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, metadataTableName, metadataTableName+"_id_idx", + ).Scan(&sql) + require.NoError(t, err) + + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, + metadataTableName+"_id_idx", dbName, metadataTableName, + ) + require.Equal(t, expected, sql) + + err = db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, metadataTableName, metadataTableName+"_table_idx", + ).Scan(&sql) + assert.NoError(t, err) + + expected = fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'table'::text)))`, + metadataTableName+"_id_idx", dbName, metadataTableName, + ) + assert.Equal(t, expected, sql) + }) } From ccc1b978760e8168a5b0f7b28db29a662bbc156f Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 27 Sep 2023 14:46:46 +0200 Subject: [PATCH 08/39] wip --- .../backends/postgresql/metadata/metadata.go | 28 ++++++++++++++-- .../backends/postgresql/metadata/registry.go | 3 +- .../postgresql/metadata/registry_test.go | 32 +++++++++---------- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 725a6850f80e..31922ad95a63 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -121,7 +121,7 @@ func (s Settings) Marshal() *types.Document { indexes.Append(must.NotFail(types.NewDocument( "name", index.Name, - "dbindex", index.TableIndexName, + "pgindex", index.TableIndexName, "key", key, "unique", index.Unique, ))) @@ -153,10 +153,32 @@ func (s *Settings) Unmarshal(doc *types.Document) error { doc := v.(*types.Document) + key := make([]IndexKeyPair, doc.Len()) + keyIter := must.NotFail(doc.Get("key")).(*types.Array).Iterator() + defer keyIter.Close() + + for { + j, el, err := keyIter.Next() + if errors.Is(err, iterator.ErrIteratorDone) { + break + } + + if err != nil { + return lazyerrors.Error(err) + } + + eldoc := el.(*types.Document) + + key[j] = IndexKeyPair{ + Field: must.NotFail(eldoc.Get("field")).(string), + Descending: must.NotFail(eldoc.Get("descending")).(bool), + } + } + s.Indexes[i] = IndexInfo{ Name: must.NotFail(doc.Get("name")).(string), - TableIndexName: must.NotFail(doc.Get("dbindex")).(string), - Key: must.NotFail(doc.Get("key")).([]IndexKeyPair), + TableIndexName: must.NotFail(doc.Get("pgindex")).(string), + Key: key, Unique: must.NotFail(doc.Get("unique")).(bool), } } diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 8250d25ecd7c..0f0740702f06 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -766,8 +766,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c h := fnv.New32a() must.NotFail(h.Write([]byte(index.Name))) s := h.Sum32() - dbIndex := fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) - index.TableIndexName = dbIndex + index.TableIndexName = fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) q := "CREATE " diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index b82099d639ad..86e55d95d9b9 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -476,69 +476,69 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) t.Run("NonUniqueIndex", func(t *testing.T) { - dbIndex := "index_non_unique_13064d07" + tableIndexName := "index_non_unique_13064d07" var sql string err = db.QueryRow( ctx, "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, dbIndex, + dbName, collection.TableName, tableIndexName, ).Scan(&sql) require.NoError(t, err) expected := fmt.Sprintf( `CREATE INDEX %s ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, - dbIndex, dbName, collection.TableName, + tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) t.Run("UniqueIndex", func(t *testing.T) { - dbIndex := "index_unique_d29d5863" + tableIndexName := "index_unique_d29d5863" var sql string err = db.QueryRow( ctx, "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, dbIndex, + dbName, collection.TableName, tableIndexName, ).Scan(&sql) require.NoError(t, err) expected := fmt.Sprintf( `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, - dbIndex, dbName, collection.TableName, + tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) t.Run("NestedFields", func(t *testing.T) { - dbIndex := "nested_fields_21fa0586" + tableIndexName := "nested_fields_21fa0586" var sql string err = db.QueryRow( ctx, "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, dbIndex, + dbName, collection.TableName, tableIndexName, ).Scan(&sql) require.NoError(t, err) expected := fmt.Sprintf( `CREATE INDEX %s ON %q.%s USING btree ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, - dbIndex, dbName, collection.TableName, + tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) t.Run("DefaultIndex", func(t *testing.T) { - dbIndex := "_id__67399184" + tableIndexName := "_id__67399184" var sql string err = db.QueryRow( ctx, "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, dbIndex, + dbName, collection.TableName, tableIndexName, ).Scan(&sql) require.NoError(t, err) expected := fmt.Sprintf( `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, - dbIndex, dbName, collection.TableName, + tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) @@ -549,11 +549,11 @@ func TestIndexesCreateDrop(t *testing.T) { collection, err = r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - require.Equal(t, 3, len(collection.Settings.Indexes)) + require.Equal(t, 4, len(collection.Settings.Indexes)) }) t.Run("DropIndexes", func(t *testing.T) { - toDrop := []string{"index_non_unique", "index_unique"} + toDrop := []string{"index_non_unique", "nested_fields"} err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) require.NoError(t, err) @@ -571,7 +571,7 @@ func TestIndexesCreateDrop(t *testing.T) { collection, err = r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - require.Equal(t, 1, len(collection.Settings.Indexes)) + require.Equal(t, 2, len(collection.Settings.Indexes)) }) t.Run("MetadataIndexes", func(t *testing.T) { @@ -600,7 +600,7 @@ func TestIndexesCreateDrop(t *testing.T) { expected = fmt.Sprintf( `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'table'::text)))`, - metadataTableName+"_id_idx", dbName, metadataTableName, + metadataTableName+"_table_idx", dbName, metadataTableName, ) assert.Equal(t, expected, sql) }) From 97e0a21e840ba6f83d4d25920687257f65c4c676 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 27 Sep 2023 19:24:09 +0200 Subject: [PATCH 09/39] fixed a strange thing --- internal/backends/postgresql/metadata/registry.go | 3 ++- internal/backends/postgresql/metadata/registry_test.go | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 0f0740702f06..bd39302a11cb 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -766,7 +766,8 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c h := fnv.New32a() must.NotFail(h.Write([]byte(index.Name))) s := h.Sum32() - index.TableIndexName = fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) + tableIndexName := fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) + index.TableIndexName = tableIndexName q := "CREATE " diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 86e55d95d9b9..3c7c7b35ae7f 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -427,8 +427,7 @@ func TestIndexesCreateDrop(t *testing.T) { collectionName := testutil.CollectionName(t) t.Cleanup(func() { - _, err := r.DatabaseDrop(ctx, dbName) - require.NoError(t, err) + _, _ = r.DatabaseDrop(ctx, dbName) }) toCreate := []IndexInfo{ From e97422bb0e5973188e6d559ec903ccf08b5c8d5c Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Thu, 28 Sep 2023 15:57:37 +0200 Subject: [PATCH 10/39] bugfix --- internal/backends/postgresql/metadata/metadata.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 31922ad95a63..82b4d7c87b6f 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -94,9 +94,10 @@ func (s Settings) deepCopy() Settings { for i, index := range s.Indexes { indexes[i] = IndexInfo{ - Name: index.Name, - Key: slices.Clone(index.Key), - Unique: index.Unique, + Name: index.Name, + TableIndexName: index.TableIndexName, + Key: slices.Clone(index.Key), + Unique: index.Unique, } } From cdaca09d11fdff5f761ff0435cff9f4aa698f8f7 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Thu, 28 Sep 2023 16:00:14 +0200 Subject: [PATCH 11/39] fix test --- internal/backends/postgresql/metadata/registry_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 3c7c7b35ae7f..1e319f9eab67 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -556,12 +556,12 @@ func TestIndexesCreateDrop(t *testing.T) { err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) require.NoError(t, err) - q := "SELECT count(*) FROM sqlite_master WHERE type = 'index' AND tbl_name = ?" - row := db.QueryRow(ctx, q, collection.TableName) + q := "SELECT count(indexdef) FROM pg_indexes WHERE schemaname = $1 AND tablename = $2" + row := db.QueryRow(ctx, q, dbName, collection.TableName) var count int require.NoError(t, row.Scan(&count)) - require.Equal(t, 1, count) // only default index + require.Equal(t, 2, count) // only default index and index_unique should be left }) t.Run("CheckSettingsAfterDrop", func(t *testing.T) { From a198a14a1ac204484557f3a3b8b969c26521f37f Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Thu, 28 Sep 2023 16:11:41 +0200 Subject: [PATCH 12/39] linter things --- internal/backends/postgresql/metadata/metadata.go | 1 + internal/backends/postgresql/metadata/registry.go | 12 ++++++++++-- .../backends/postgresql/metadata/registry_test.go | 6 +++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 82b4d7c87b6f..f417e94191da 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -156,6 +156,7 @@ func (s *Settings) Unmarshal(doc *types.Document) error { key := make([]IndexKeyPair, doc.Len()) keyIter := must.NotFail(doc.Get("key")).(*types.Array).Iterator() + defer keyIter.Close() for { diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index bd39302a11cb..983bd1c87766 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -339,16 +339,19 @@ func (r *Registry) databaseGetOrCreate(ctx context.Context, p *pgxpool.Pool, dbN pgx.Identifier{dbName, metadataTableName}.Sanitize(), IDColumn, ) + if _, err = p.Exec(ctx, q); err != nil { _, _ = r.databaseDrop(ctx, p, dbName) return nil, lazyerrors.Error(err) } + q = fmt.Sprintf( `CREATE UNIQUE INDEX %s ON %s (((%s->'table')))`, pgx.Identifier{metadataTableName + "_table_idx"}.Sanitize(), pgx.Identifier{dbName, metadataTableName}.Sanitize(), DefaultColumn, ) + if _, err = p.Exec(ctx, q); err != nil { _, _ = r.databaseDrop(ctx, p, dbName) return nil, lazyerrors.Error(err) @@ -778,8 +781,8 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c q += "INDEX %s ON %s (%s)" columns := make([]string, len(index.Key)) - for i, key := range index.Key { + for i, key := range index.Key { // if the field is nested (e.g. foo.bar), it needs to be translated to the correct json path (foo -> bar) fs := strings.Split(key.Field, ".") transformedParts := make([]string, len(fs)) @@ -795,7 +798,12 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c } } - q = fmt.Sprintf(q, pgx.Identifier{index.TableIndexName}.Sanitize(), pgx.Identifier{dbName, c.TableName}.Sanitize(), strings.Join(columns, ", ")) + q = fmt.Sprintf( + q, + pgx.Identifier{index.TableIndexName}.Sanitize(), + pgx.Identifier{dbName, c.TableName}.Sanitize(), + strings.Join(columns, ", "), + ) if _, err = p.Exec(ctx, q); err != nil { _ = r.indexesDrop(ctx, p, dbName, collectionName, created) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 1e319f9eab67..1040ffafc28e 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -20,10 +20,9 @@ import ( "sync/atomic" "testing" - "github.com/stretchr/testify/assert" - "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" @@ -519,7 +518,8 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE INDEX %s ON %q.%s USING btree ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, + `CREATE INDEX %s ON %q.%s USING btree`+ + ` ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) From 49ece35319798ca7f6f4565bdd3a448de6abb6bd Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 00:39:55 +0200 Subject: [PATCH 13/39] wip --- .../backends/postgresql/metadata/registry.go | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 4b87574139da..017f2bb0d2ef 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -49,6 +49,9 @@ const ( // PostgreSQL max table name length. maxTableNameLength = 63 + + // PostgreSQL max index name length. + maxIndexNameLength = 63 ) // Parts of Prometheus metric names. @@ -783,7 +786,34 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c h := fnv.New32a() must.NotFail(h.Write([]byte(index.Name))) s := h.Sum32() - tableIndexName := fmt.Sprintf("%s_%08x", strings.ToLower(index.Name), s) + + var tableIndexName string + + // To create index in the DB, we truncate the index name to maxIndexNameLength. + // If the truncated name is already used, we increment the hash and try again. + for { + tableIndexName = fmt.Sprintf("%s_%s_idx", strings.ToLower(collectionName), strings.ToLower(index.Name)) + tableIndexName = specialCharacters.ReplaceAllString(tableIndexName, "_") + + if strings.HasPrefix(tableIndexName, reservedPrefix) { + tableIndexName = "_" + tableIndexName + } + + suffixHash := fmt.Sprintf("_%08x", s) + if l := maxIndexNameLength - len(suffixHash); len(tableIndexName) > l { + tableIndexName = tableIndexName[:l] + } + + tableIndexName = fmt.Sprintf("%s%s", tableIndexName, suffixHash) + + if !slices.ContainsFunc(c.Settings.Indexes, func(ii IndexInfo) bool { return ii.TableIndexName == tableIndexName }) { + break + } + + // the given name is already used, generate a new name by incrementing the hash + s++ + } + index.TableIndexName = tableIndexName q := "CREATE " From ed53d662fc985f17e1fa82258da7678162392c94 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 10:12:25 +0200 Subject: [PATCH 14/39] wip --- .../backends/postgresql/metadata/metadata.go | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index f417e94191da..b7011bc3bedd 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -111,18 +111,23 @@ func (s Settings) Marshal() *types.Document { indexes := types.MakeArray(len(s.Indexes)) for _, index := range s.Indexes { - key := types.MakeArray(len(index.Key)) + key := types.MakeDocument(len(index.Key)) + // The format of the index key storing was defined in the early versions of FerretDB, + // it's kept for backward compatibility. for _, pair := range index.Key { - key.Append(must.NotFail(types.NewDocument( - "field", pair.Field, - "descending", pair.Descending, - ))) + order := int32(1) // order is set as int32 to be sjson-marshaled correctly + + if pair.Descending { + order = -1 + } + + key.Set(pair.Field, order) } indexes.Append(must.NotFail(types.NewDocument( - "name", index.Name, "pgindex", index.TableIndexName, + "name", index.Name, "key", key, "unique", index.Unique, ))) @@ -155,12 +160,12 @@ func (s *Settings) Unmarshal(doc *types.Document) error { doc := v.(*types.Document) key := make([]IndexKeyPair, doc.Len()) - keyIter := must.NotFail(doc.Get("key")).(*types.Array).Iterator() + keyIter := must.NotFail(doc.Get("key")).(*types.Document).Iterator() defer keyIter.Close() - for { - j, el, err := keyIter.Next() + for j := 0; ; j++ { + field, order, err := keyIter.Next() if errors.Is(err, iterator.ErrIteratorDone) { break } @@ -169,11 +174,14 @@ func (s *Settings) Unmarshal(doc *types.Document) error { return lazyerrors.Error(err) } - eldoc := el.(*types.Document) + descending := false + if order.(int32) == -1 { + descending = true + } key[j] = IndexKeyPair{ - Field: must.NotFail(eldoc.Get("field")).(string), - Descending: must.NotFail(eldoc.Get("descending")).(bool), + Field: field, + Descending: descending, } } From 184ec9ac74843414fc714383653f4aa060ea4f62 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 10:14:04 +0200 Subject: [PATCH 15/39] wip --- internal/backends/postgresql/metadata/metadata.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index b7011bc3bedd..f2b922adf7b1 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -198,14 +198,14 @@ func (s *Settings) Unmarshal(doc *types.Document) error { // IndexInfo represents information about a single index. type IndexInfo struct { - Name string `json:"name"` - TableIndexName string `json:"pgindex"` // how the index is created in the DB, like TableName for Collection - Key []IndexKeyPair `json:"key"` - Unique bool `json:"unique"` + Name string + TableIndexName string // how the index is created in the DB, like TableName for Collection + Key []IndexKeyPair + Unique bool } // IndexKeyPair consists of a field name and a sort order that are part of the index. type IndexKeyPair struct { - Field string `json:"field"` - Descending bool `json:"descending"` + Field string + Descending bool } From c1e2861d21067ba3dfa93a8a004460e6d3e269f4 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 11:53:11 +0200 Subject: [PATCH 16/39] wip --- .../backends/postgresql/metadata/registry.go | 37 ++++-------------- .../postgresql/metadata/registry_test.go | 38 +++++++++++++++---- 2 files changed, 38 insertions(+), 37 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 017f2bb0d2ef..af1d5c221a34 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -24,6 +24,8 @@ import ( "strings" "sync" + "github.com/google/uuid" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/prometheus/client_golang/prometheus" @@ -783,38 +785,15 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c continue } - h := fnv.New32a() - must.NotFail(h.Write([]byte(index.Name))) - s := h.Sum32() - - var tableIndexName string - - // To create index in the DB, we truncate the index name to maxIndexNameLength. - // If the truncated name is already used, we increment the hash and try again. - for { - tableIndexName = fmt.Sprintf("%s_%s_idx", strings.ToLower(collectionName), strings.ToLower(index.Name)) - tableIndexName = specialCharacters.ReplaceAllString(tableIndexName, "_") - - if strings.HasPrefix(tableIndexName, reservedPrefix) { - tableIndexName = "_" + tableIndexName - } - - suffixHash := fmt.Sprintf("_%08x", s) - if l := maxIndexNameLength - len(suffixHash); len(tableIndexName) > l { - tableIndexName = tableIndexName[:l] - } - - tableIndexName = fmt.Sprintf("%s%s", tableIndexName, suffixHash) - - if !slices.ContainsFunc(c.Settings.Indexes, func(ii IndexInfo) bool { return ii.TableIndexName == tableIndexName }) { - break - } + uuidPart := must.NotFail(uuid.NewRandom()).String() + tableNamePart := c.TableName + tableNamePartMax := maxIndexNameLength - len(uuidPart) - 5 // 5 is for _ and _idx - // the given name is already used, generate a new name by incrementing the hash - s++ + if len(tableNamePart) > tableNamePartMax { + tableNamePart = tableNamePart[:tableNamePartMax] } - index.TableIndexName = tableIndexName + index.TableIndexName = fmt.Sprintf("%s_%s_idx", tableNamePart, uuidPart) q := "CREATE " diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 43eb054757db..b0a17edb26b1 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -21,6 +21,8 @@ import ( "sync/atomic" "testing" + "golang.org/x/exp/slices" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" @@ -504,7 +506,12 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) t.Run("NonUniqueIndex", func(t *testing.T) { - tableIndexName := "index_non_unique_13064d07" + i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + return ii.Name == "index_non_unique" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Settings.Indexes[i].TableIndexName + var sql string err = db.QueryRow( ctx, @@ -514,14 +521,19 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE INDEX %s ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, + `CREATE INDEX %q ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) t.Run("UniqueIndex", func(t *testing.T) { - tableIndexName := "index_unique_d29d5863" + i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + return ii.Name == "index_unique" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Settings.Indexes[i].TableIndexName + var sql string err = db.QueryRow( ctx, @@ -531,14 +543,19 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, + `CREATE UNIQUE INDEX %q ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) }) t.Run("NestedFields", func(t *testing.T) { - tableIndexName := "nested_fields_21fa0586" + i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + return ii.Name == "nested_fields" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Settings.Indexes[i].TableIndexName + var sql string err = db.QueryRow( ctx, @@ -548,7 +565,7 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE INDEX %s ON %q.%s USING btree`+ + `CREATE INDEX %q ON %q.%s USING btree`+ ` ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, tableIndexName, dbName, collection.TableName, ) @@ -556,7 +573,12 @@ func TestIndexesCreateDrop(t *testing.T) { }) t.Run("DefaultIndex", func(t *testing.T) { - tableIndexName := "_id__67399184" + i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + return ii.Name == "_id_" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Settings.Indexes[i].TableIndexName + var sql string err = db.QueryRow( ctx, @@ -566,7 +588,7 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, + `CREATE UNIQUE INDEX %q ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) From 0d7bf077063b9a762873993434ced35f0e7d4eb8 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 11:58:47 +0200 Subject: [PATCH 17/39] wip --- internal/backends/postgresql/metadata/registry.go | 1 + internal/backends/postgresql/metadata/registry_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index af1d5c221a34..4f3f07f3019c 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -785,6 +785,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c continue } + // indexes must be unique across the whole database, so we add a uuid to the index name uuidPart := must.NotFail(uuid.NewRandom()).String() tableNamePart := c.TableName tableNamePartMax := maxIndexNameLength - len(uuidPart) - 5 // 5 is for _ and _idx diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index b0a17edb26b1..e695121f9eb2 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -440,6 +440,7 @@ func TestRenameCollection(t *testing.T) { expected := &Collection{ Name: newCollectionName, TableName: oldCollection.TableName, + Settings: oldCollection.Settings, } actual, err := r.CollectionGet(ctx, dbName, newCollectionName) From 586f314528e5ef03e449310fbafdb98af53edf05 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 12:04:08 +0200 Subject: [PATCH 18/39] wip --- .../backends/postgresql/metadata/registry_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index e695121f9eb2..b844276fc429 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -624,6 +624,17 @@ func TestIndexesCreateDrop(t *testing.T) { collection, err = r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) require.Equal(t, 2, len(collection.Settings.Indexes)) + + for _, index := range collection.Settings.Indexes { + switch index.Name { + case "_id_": + assert.Equal(t, 1, len(index.Key)) + case "index_unique": + assert.Equal(t, 1, len(index.Key)) + default: + t.Errorf("unexpected index: %s", index.Name) + } + } }) t.Run("MetadataIndexes", func(t *testing.T) { From 3661271e3013393256d7e3f8f79e8bc7d750978e Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 12:08:04 +0200 Subject: [PATCH 19/39] wip --- internal/backends/postgresql/metadata/registry_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index b844276fc429..4a583f16889b 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -615,9 +615,8 @@ func TestIndexesCreateDrop(t *testing.T) { var count int require.NoError(t, row.Scan(&count)) require.Equal(t, 2, count) // only default index and index_unique should be left - }) - t.Run("CheckSettingsAfterDrop", func(t *testing.T) { + // check settings after dropping indexes err = r.initCollections(ctx, dbName, db) require.NoError(t, err) From 9a9a5a824f86b1b9a7ff09752418a70a046aaf41 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 12:26:48 +0200 Subject: [PATCH 20/39] wip --- .../backends/postgresql/metadata/metadata.go | 5 +++-- .../postgresql/metadata/registry_test.go | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index f2b922adf7b1..ac1e913135d4 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -159,8 +159,9 @@ func (s *Settings) Unmarshal(doc *types.Document) error { doc := v.(*types.Document) - key := make([]IndexKeyPair, doc.Len()) - keyIter := must.NotFail(doc.Get("key")).(*types.Document).Iterator() + keyDoc := must.NotFail(doc.Get("key")).(*types.Document) + keyIter := keyDoc.Iterator() + key := make([]IndexKeyPair, keyDoc.Len()) defer keyIter.Close() diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 4a583f16889b..6ba7590fecf2 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -599,9 +599,25 @@ func TestIndexesCreateDrop(t *testing.T) { err = r.initCollections(ctx, dbName, db) require.NoError(t, err) - collection, err = r.CollectionGet(ctx, dbName, collectionName) + refreshedCollection, err := r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - require.Equal(t, 4, len(collection.Settings.Indexes)) + + require.Equal(t, 4, len(refreshedCollection.Settings.Indexes)) + + for _, index := range refreshedCollection.Settings.Indexes { + switch { + case index.Name == "_id_": + assert.Equal(t, 1, len(index.Key)) + case index.Name == "index_non_unique": + assert.Equal(t, 2, len(index.Key)) + case index.Name == "index_unique": + assert.Equal(t, 1, len(index.Key)) + case index.Name == "nested_fields": + assert.Equal(t, 2, len(index.Key)) + default: + t.Errorf("unexpected index: %s", index.Name) + } + } }) t.Run("DropIndexes", func(t *testing.T) { From 97afd35e07d76ee1142cd0844869321cae8d34fb Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 12:46:41 +0200 Subject: [PATCH 21/39] wip --- internal/backends/postgresql/metadata/registry_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 6ba7590fecf2..108cd22d7a94 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -409,6 +409,8 @@ func TestRenameCollection(t *testing.T) { t.Skip("skipping in -short mode") } + t.Skip("Rename collection doesn't work correctly") + t.Parallel() connInfo := conninfo.New() From db39ee528c0d5053eb56b746edbdeccfdd26bb8c Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 12:50:52 +0200 Subject: [PATCH 22/39] wip --- internal/backends/postgresql/metadata/registry.go | 1 - internal/backends/postgresql/metadata/registry_test.go | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 4f3f07f3019c..477ae819632f 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -25,7 +25,6 @@ import ( "sync" "github.com/google/uuid" - "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/prometheus/client_golang/prometheus" diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 108cd22d7a94..11ff4e567010 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -21,12 +21,11 @@ import ( "sync/atomic" "testing" - "golang.org/x/exp/slices" - "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/util/state" @@ -409,7 +408,7 @@ func TestRenameCollection(t *testing.T) { t.Skip("skipping in -short mode") } - t.Skip("Rename collection doesn't work correctly") + t.Skip("https://github.com/FerretDB/FerretDB/issues/3409") t.Parallel() @@ -601,7 +600,8 @@ func TestIndexesCreateDrop(t *testing.T) { err = r.initCollections(ctx, dbName, db) require.NoError(t, err) - refreshedCollection, err := r.CollectionGet(ctx, dbName, collectionName) + var refreshedCollection *Collection + refreshedCollection, err = r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) require.Equal(t, 4, len(refreshedCollection.Settings.Indexes)) From 05139bb069f4047d9b48662379062d2640dbda80 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 12:58:30 +0200 Subject: [PATCH 23/39] wip --- internal/backends/postgresql/metadata/metadata.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index ac1e913135d4..77011ff951b5 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -71,14 +71,9 @@ func (c *Collection) Unmarshal(doc *types.Document) error { c.Name = must.NotFail(doc.Get("_id")).(string) c.TableName = must.NotFail(doc.Get("table")).(string) - if doc.Has("settings") { - var settings Settings - must.NoError(settings.Unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) - c.Settings = settings - } else { - // If settings are not present, we initialize them with empty indexes to avoid potential nil pointers. - c.Settings = Settings{Indexes: []IndexInfo{}} - } + var settings Settings + must.NoError(settings.Unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) + c.Settings = settings return nil } From 0fb411ace398a02139b75f9c1206461352e2d203 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 15:44:16 +0200 Subject: [PATCH 24/39] wip --- internal/backends/postgresql/metadata/registry.go | 8 ++++---- internal/backends/sqlite/metadata/registry.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 477ae819632f..db5fa4c6d22b 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -739,7 +739,7 @@ func (r *Registry) CollectionRename(ctx context.Context, dbName, oldCollectionNa // IndexesCreate creates indexes in the collection. // -// Existing indexes with given names are ignored (TODO?). +// Existing indexes with given names are ignored. func (r *Registry) IndexesCreate(ctx context.Context, dbName, collectionName string, indexes []IndexInfo) error { defer observability.FuncCall(ctx)() @@ -756,7 +756,7 @@ func (r *Registry) IndexesCreate(ctx context.Context, dbName, collectionName str // indexesCreate creates indexes in the collection. // -// Existing indexes with given names are ignored (TODO?). +// Existing indexes with given names are ignored. // // It does not hold the lock. func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, collectionName string, indexes []IndexInfo) error { @@ -880,9 +880,9 @@ func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName strin // indexesDrop remove given connection's indexes. // -// Non-existing indexes are ignored (TODO?). +// Non-existing indexes are ignored. // -// If database or collection does not exist, nil is returned (TODO?). +// If database or collection does not exist, nil is returned. // // It does not hold the lock. func (r *Registry) indexesDrop(ctx context.Context, p *pgxpool.Pool, dbName, collectionName string, indexNames []string) error { diff --git a/internal/backends/sqlite/metadata/registry.go b/internal/backends/sqlite/metadata/registry.go index 3bc5dfc7c3c5..53e222081ff6 100644 --- a/internal/backends/sqlite/metadata/registry.go +++ b/internal/backends/sqlite/metadata/registry.go @@ -427,7 +427,7 @@ func (r *Registry) CollectionRename(ctx context.Context, dbName, oldCollectionNa // IndexesCreate creates indexes in the collection. // -// Existing indexes with given names are ignored (TODO?). +// Existing indexes with given names are ignored. func (r *Registry) IndexesCreate(ctx context.Context, dbName, collectionName string, indexes []IndexInfo) error { defer observability.FuncCall(ctx)() @@ -439,7 +439,7 @@ func (r *Registry) IndexesCreate(ctx context.Context, dbName, collectionName str // indexesCreate creates indexes in the collection. // -// Existing indexes with given names are ignored (TODO?). +// Existing indexes with given names are ignored. // // It does not hold the lock. func (r *Registry) indexesCreate(ctx context.Context, dbName, collectionName string, indexes []IndexInfo) error { @@ -518,9 +518,9 @@ func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName strin // indexesDrop remove given connection's indexes. // -// Non-existing indexes are ignored (TODO?). +// Non-existing indexes are ignored. // -// If database or collection does not exist, nil is returned (TODO?). +// If database or collection does not exist, nil is returned. // // It does not hold the lock. func (r *Registry) indexesDrop(ctx context.Context, dbName, collectionName string, indexNames []string) error { From 71c838907691844fb357f066e5f8bc0a9de0f865 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 23:16:04 +0200 Subject: [PATCH 25/39] wip --- internal/backends/postgresql/metadata/registry_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 11ff4e567010..b27958708794 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -459,10 +459,6 @@ func TestIndexesCreateDrop(t *testing.T) { r, db, dbName := createDatabase(t, ctx) collectionName := testutil.CollectionName(t) - t.Cleanup(func() { - _, _ = r.DatabaseDrop(ctx, dbName) - }) - toCreate := []IndexInfo{ { Name: "index_non_unique", From e577da1e81c39a181d22b0e48d83143f4ca0f086 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Fri, 29 Sep 2023 23:43:10 +0200 Subject: [PATCH 26/39] wip --- internal/backends/postgresql/metadata/registry_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index b27958708794..aa4017cddbcd 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -451,6 +451,10 @@ func TestRenameCollection(t *testing.T) { } func TestIndexesCreateDrop(t *testing.T) { + if testing.Short() { + t.Skip("skipping in -short mode") + } + t.Parallel() connInfo := conninfo.New() From ebe36f1221a1b441d55235955a6a27653dc03fbe Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Mon, 2 Oct 2023 14:39:07 +0400 Subject: [PATCH 27/39] Simplify --- internal/backends/postgresql/metadata/registry.go | 2 +- internal/util/state/state.go | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index db5fa4c6d22b..5795468ae588 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -785,7 +785,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c } // indexes must be unique across the whole database, so we add a uuid to the index name - uuidPart := must.NotFail(uuid.NewRandom()).String() + uuidPart := uuid.NewString() tableNamePart := c.TableName tableNamePartMax := maxIndexNameLength - len(uuidPart) - 5 // 5 is for _ and _idx diff --git a/internal/util/state/state.go b/internal/util/state/state.go index 578f258ae9bb..c06397710bd8 100644 --- a/internal/util/state/state.go +++ b/internal/util/state/state.go @@ -20,8 +20,6 @@ import ( "github.com/AlekSi/pointer" "github.com/google/uuid" - - "github.com/FerretDB/FerretDB/internal/util/must" ) // State represents FerretDB process state. @@ -68,7 +66,7 @@ func (s *State) EnableTelemetry() { // fill replaces all unset or invalid values with default. func (s *State) fill() { if _, err := uuid.Parse(s.UUID); err != nil { - s.UUID = must.NotFail(uuid.NewRandom()).String() + s.UUID = uuid.NewString() } if s.Start.IsZero() { From 0f6cf2c0295f8715bff6146f1c4a6b7b5f7c1ae0 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Mon, 2 Oct 2023 14:49:20 +0400 Subject: [PATCH 28/39] Tidy comments --- .../backends/postgresql/metadata/registry.go | 16 ++++++++++++---- internal/backends/sqlite/metadata/registry.go | 12 ++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 5795468ae588..94fe529c7ac8 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -740,6 +740,8 @@ func (r *Registry) CollectionRename(ctx context.Context, dbName, oldCollectionNa // IndexesCreate creates indexes in the collection. // // Existing indexes with given names are ignored. +// +// If the user is not authenticated, it returns error. func (r *Registry) IndexesCreate(ctx context.Context, dbName, collectionName string, indexes []IndexInfo) error { defer observability.FuncCall(ctx)() @@ -863,8 +865,14 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c return nil } -// IndexesDrop drops provided indexes for the given collection. -func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName string, toDrop []string) error { +// IndexesDrop removes given connection's indexes. +// +// Non-existing indexes are ignored. +// +// If database or collection does not exist, nil is returned. +// +// If the user is not authenticated, it returns error. +func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName string, indexNames []string) error { defer observability.FuncCall(ctx)() p, err := r.getPool(ctx) @@ -875,10 +883,10 @@ func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName strin r.rw.Lock() defer r.rw.Unlock() - return r.indexesDrop(ctx, p, dbName, collectionName, toDrop) + return r.indexesDrop(ctx, p, dbName, collectionName, indexNames) } -// indexesDrop remove given connection's indexes. +// indexesDrop removes given connection's indexes. // // Non-existing indexes are ignored. // diff --git a/internal/backends/sqlite/metadata/registry.go b/internal/backends/sqlite/metadata/registry.go index 53e222081ff6..461eb8db3b4a 100644 --- a/internal/backends/sqlite/metadata/registry.go +++ b/internal/backends/sqlite/metadata/registry.go @@ -506,17 +506,21 @@ func (r *Registry) indexesCreate(ctx context.Context, dbName, collectionName str return nil } -// IndexesDrop drops provided indexes for the given collection. -func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName string, toDrop []string) error { +// IndexesDrop removes given connection's indexes. +// +// Non-existing indexes are ignored. +// +// If database or collection does not exist, nil is returned. +func (r *Registry) IndexesDrop(ctx context.Context, dbName, collectionName string, indexNames []string) error { defer observability.FuncCall(ctx)() r.rw.Lock() defer r.rw.Unlock() - return r.indexesDrop(ctx, dbName, collectionName, toDrop) + return r.indexesDrop(ctx, dbName, collectionName, indexNames) } -// indexesDrop remove given connection's indexes. +// indexesDrop removes given connection's indexes. // // Non-existing indexes are ignored. // From f26890179dafabff3ecf393d64853ee79254b2e8 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Mon, 2 Oct 2023 17:48:55 +0400 Subject: [PATCH 29/39] Extract settings --- .../backends/postgresql/metadata/metadata.go | 193 ++++++------------ .../backends/postgresql/metadata/registry.go | 49 ++--- .../postgresql/metadata/registry_test.go | 8 +- .../backends/postgresql/metadata/settings.go | 154 ++++++++++++++ internal/backends/sqlite/metadata/metadata.go | 83 -------- internal/backends/sqlite/metadata/settings.go | 98 +++++++++ 6 files changed, 331 insertions(+), 254 deletions(-) create mode 100644 internal/backends/postgresql/metadata/settings.go create mode 100644 internal/backends/sqlite/metadata/settings.go diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index 77011ff951b5..e7ff3472f396 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -16,12 +16,11 @@ package metadata import ( - "errors" - - "golang.org/x/exp/slices" + "database/sql" + "database/sql/driver" + "github.com/FerretDB/FerretDB/internal/handlers/sjson" "github.com/FerretDB/FerretDB/internal/types" - "github.com/FerretDB/FerretDB/internal/util/iterator" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" ) @@ -57,151 +56,83 @@ func (c *Collection) deepCopy() *Collection { } } -// Marshal returns [*types.Document] for that collection. -func (c *Collection) Marshal() *types.Document { - return must.NotFail(types.NewDocument( - "_id", c.Name, - "table", c.TableName, - "settings", c.Settings.Marshal(), - )) -} - -// Unmarshal sets collection metadata from [*types.Document]. -func (c *Collection) Unmarshal(doc *types.Document) error { - c.Name = must.NotFail(doc.Get("_id")).(string) - c.TableName = must.NotFail(doc.Get("table")).(string) - - var settings Settings - must.NoError(settings.Unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) - c.Settings = settings - - return nil -} +// Value implements driver.Valuer interface. +func (c Collection) Value() (driver.Value, error) { + b, err := sjson.Marshal(c.marshal()) + if err != nil { + return nil, lazyerrors.Error(err) + } -// Settings represents collection settings. -type Settings struct { - Indexes []IndexInfo `json:"indexes"` + return b, nil } -// deepCopy returns a deep copy. -func (s Settings) deepCopy() Settings { - indexes := make([]IndexInfo, len(s.Indexes)) - - for i, index := range s.Indexes { - indexes[i] = IndexInfo{ - Name: index.Name, - TableIndexName: index.TableIndexName, - Key: slices.Clone(index.Key), - Unique: index.Unique, - } +// Scan implements sql.Scanner interface. +func (c *Collection) Scan(src any) error { + var doc *types.Document + var err error + switch src := src.(type) { + case nil: + *c = Collection{} + return nil + case []byte: + doc, err = sjson.Unmarshal(src) + case string: + doc, err = sjson.Unmarshal([]byte(src)) + default: + panic("can't scan collection") } - return Settings{ - Indexes: indexes, + if err != nil { + return lazyerrors.Error(err) } -} - -// Marshal returns [*types.Document] for settings. -func (s Settings) Marshal() *types.Document { - indexes := types.MakeArray(len(s.Indexes)) - for _, index := range s.Indexes { - key := types.MakeDocument(len(index.Key)) - - // The format of the index key storing was defined in the early versions of FerretDB, - // it's kept for backward compatibility. - for _, pair := range index.Key { - order := int32(1) // order is set as int32 to be sjson-marshaled correctly - - if pair.Descending { - order = -1 - } - - key.Set(pair.Field, order) - } - - indexes.Append(must.NotFail(types.NewDocument( - "pgindex", index.TableIndexName, - "name", index.Name, - "key", key, - "unique", index.Unique, - ))) + if err = c.unmarshal(doc); err != nil { + return lazyerrors.Error(err) } + return nil +} + +// marshal returns [*types.Document] for that collection. +func (c *Collection) marshal() *types.Document { return must.NotFail(types.NewDocument( - "indexes", indexes, + "_id", c.Name, + "table", c.TableName, + "settings", c.Settings.marshal(), )) } -// Unmarshal sets settings from [*types.Document]. -func (s *Settings) Unmarshal(doc *types.Document) error { - indexes := must.NotFail(doc.Get("indexes")).(*types.Array) - - s.Indexes = make([]IndexInfo, indexes.Len()) - - iter := indexes.Iterator() - defer iter.Close() - - for { - i, v, err := iter.Next() - if errors.Is(err, iterator.ErrIteratorDone) { - break - } - - if err != nil { - return lazyerrors.Error(err) - } - - doc := v.(*types.Document) - - keyDoc := must.NotFail(doc.Get("key")).(*types.Document) - keyIter := keyDoc.Iterator() - key := make([]IndexKeyPair, keyDoc.Len()) - - defer keyIter.Close() - - for j := 0; ; j++ { - field, order, err := keyIter.Next() - if errors.Is(err, iterator.ErrIteratorDone) { - break - } +// unmarshal sets collection metadata from [*types.Document]. +func (c *Collection) unmarshal(doc *types.Document) error { + v, _ := doc.Get("_id") + c.Name, _ = v.(string) + if c.Name == "" { + return lazyerrors.New("collection name is empty") + } - if err != nil { - return lazyerrors.Error(err) - } + v, _ = doc.Get("table") + c.TableName, _ = v.(string) + if c.TableName == "" { + return lazyerrors.New("table name is empty") + } - descending := false - if order.(int32) == -1 { - descending = true - } + v, _ = doc.Get("settings") + s, _ := v.(*types.Document) + if s == nil { + return lazyerrors.New("settings are empty") + } - key[j] = IndexKeyPair{ - Field: field, - Descending: descending, - } - } + c.Settings.unmarshal(doc) - s.Indexes[i] = IndexInfo{ - Name: must.NotFail(doc.Get("name")).(string), - TableIndexName: must.NotFail(doc.Get("pgindex")).(string), - Key: key, - Unique: must.NotFail(doc.Get("unique")).(bool), - } - } + var settings Settings + must.NoError(settings.unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) + c.Settings = settings return nil } -// IndexInfo represents information about a single index. -type IndexInfo struct { - Name string - TableIndexName string // how the index is created in the DB, like TableName for Collection - Key []IndexKeyPair - Unique bool -} - -// IndexKeyPair consists of a field name and a sort order that are part of the index. -type IndexKeyPair struct { - Field string - Descending bool -} +// check interfaces +var ( + _ driver.Valuer = Collection{} + _ sql.Scanner = (*Collection)(nil) +) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 94fe529c7ac8..44d6ba64ac0e 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -34,7 +34,6 @@ import ( "github.com/FerretDB/FerretDB/internal/backends/postgresql/metadata/pool" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/handlers/sjson" - "github.com/FerretDB/FerretDB/internal/types" "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/util/observability" @@ -221,19 +220,8 @@ func (r *Registry) initCollections(ctx context.Context, dbName string, p *pgxpoo colls := map[string]*Collection{} for rows.Next() { - var b []byte - if err = rows.Scan(&b); err != nil { - return lazyerrors.Error(err) - } - - var doc *types.Document - - if doc, err = sjson.Unmarshal(b); err != nil { - return lazyerrors.Error(err) - } - var c Collection - if err = c.Unmarshal(doc); err != nil { + if err = rows.Scan(&c); err != nil { return lazyerrors.Error(err) } @@ -525,17 +513,11 @@ func (r *Registry) collectionCreate(ctx context.Context, p *pgxpool.Pool, dbName TableName: tableName, } - b, err := sjson.Marshal(c.Marshal()) - if err != nil { - return false, lazyerrors.Error(err) - } - q := fmt.Sprintf( `CREATE TABLE %s (%s jsonb)`, pgx.Identifier{dbName, tableName}.Sanitize(), DefaultColumn, ) - if _, err = p.Exec(ctx, q); err != nil { return false, lazyerrors.Error(err) } @@ -545,8 +527,7 @@ func (r *Registry) collectionCreate(ctx context.Context, p *pgxpool.Pool, dbName pgx.Identifier{dbName, metadataTableName}.Sanitize(), DefaultColumn, ) - - if _, err = p.Exec(ctx, q, string(b)); err != nil { + if _, err = p.Exec(ctx, q, c); err != nil { q = fmt.Sprintf(`DROP TABLE %s`, pgx.Identifier{dbName, tableName}.Sanitize()) _, _ = p.Exec(ctx, q) @@ -558,15 +539,11 @@ func (r *Registry) collectionCreate(ctx context.Context, p *pgxpool.Pool, dbName } r.colls[dbName][collectionName] = c - err = r.indexesCreate(ctx, p, dbName, collectionName, - []IndexInfo{ - { - Name: "_id_", - Key: []IndexKeyPair{{Field: "_id"}}, - Unique: true, - }, - }, - ) + err = r.indexesCreate(ctx, p, dbName, collectionName, []IndexInfo{{ + Name: "_id_", + Key: []IndexKeyPair{{Field: "_id"}}, + Unique: true, + }}) if err != nil { _, _ = r.collectionDrop(ctx, p, dbName, collectionName) return false, lazyerrors.Error(err) @@ -710,7 +687,7 @@ func (r *Registry) CollectionRename(ctx context.Context, dbName, oldCollectionNa c.Name = newCollectionName - b, err := sjson.Marshal(c.Marshal()) + b, err := sjson.Marshal(c.marshal()) if err != nil { return false, lazyerrors.Error(err) } @@ -795,7 +772,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c tableNamePart = tableNamePart[:tableNamePartMax] } - index.TableIndexName = fmt.Sprintf("%s_%s_idx", tableNamePart, uuidPart) + index.PgName = fmt.Sprintf("%s_%s_idx", tableNamePart, uuidPart) q := "CREATE " @@ -825,7 +802,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c q = fmt.Sprintf( q, - pgx.Identifier{index.TableIndexName}.Sanitize(), + pgx.Identifier{index.PgName}.Sanitize(), pgx.Identifier{dbName, c.TableName}.Sanitize(), strings.Join(columns, ", "), ) @@ -839,7 +816,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c c.Settings.Indexes = append(c.Settings.Indexes, index) } - b, err := sjson.Marshal(c.Marshal()) + b, err := sjson.Marshal(c.marshal()) if err != nil { return lazyerrors.Error(err) } @@ -907,7 +884,7 @@ func (r *Registry) indexesDrop(ctx context.Context, p *pgxpool.Pool, dbName, col continue } - q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Settings.Indexes[i].TableIndexName}.Sanitize()) + q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Settings.Indexes[i].PgName}.Sanitize()) if _, err := p.Exec(ctx, q); err != nil { return lazyerrors.Error(err) } @@ -915,7 +892,7 @@ func (r *Registry) indexesDrop(ctx context.Context, p *pgxpool.Pool, dbName, col c.Settings.Indexes = slices.Delete(c.Settings.Indexes, i, i+1) } - b, err := sjson.Marshal(c.Marshal()) + b, err := sjson.Marshal(c.marshal()) if err != nil { return lazyerrors.Error(err) } diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index aa4017cddbcd..0543a4f88544 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -512,7 +512,7 @@ func TestIndexesCreateDrop(t *testing.T) { return ii.Name == "index_non_unique" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].TableIndexName + tableIndexName := collection.Settings.Indexes[i].PgName var sql string err = db.QueryRow( @@ -534,7 +534,7 @@ func TestIndexesCreateDrop(t *testing.T) { return ii.Name == "index_unique" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].TableIndexName + tableIndexName := collection.Settings.Indexes[i].PgName var sql string err = db.QueryRow( @@ -556,7 +556,7 @@ func TestIndexesCreateDrop(t *testing.T) { return ii.Name == "nested_fields" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].TableIndexName + tableIndexName := collection.Settings.Indexes[i].PgName var sql string err = db.QueryRow( @@ -579,7 +579,7 @@ func TestIndexesCreateDrop(t *testing.T) { return ii.Name == "_id_" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].TableIndexName + tableIndexName := collection.Settings.Indexes[i].PgName var sql string err = db.QueryRow( diff --git a/internal/backends/postgresql/metadata/settings.go b/internal/backends/postgresql/metadata/settings.go new file mode 100644 index 000000000000..aa6b00dca30f --- /dev/null +++ b/internal/backends/postgresql/metadata/settings.go @@ -0,0 +1,154 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metadata + +import ( + "errors" + + "golang.org/x/exp/slices" + + "github.com/FerretDB/FerretDB/internal/types" + "github.com/FerretDB/FerretDB/internal/util/iterator" + "github.com/FerretDB/FerretDB/internal/util/lazyerrors" + "github.com/FerretDB/FerretDB/internal/util/must" +) + +// Settings represents collection settings. +type Settings struct { + Indexes []IndexInfo +} + +// IndexInfo represents information about a single index. +type IndexInfo struct { + Name string + PgName string + Key []IndexKeyPair + Unique bool +} + +// IndexKeyPair consists of a field name and a sort order that are part of the index. +type IndexKeyPair struct { + Field string + Descending bool +} + +// deepCopy returns a deep copy. +func (s Settings) deepCopy() Settings { + indexes := make([]IndexInfo, len(s.Indexes)) + + for i, index := range s.Indexes { + indexes[i] = IndexInfo{ + Name: index.Name, + PgName: index.PgName, + Key: slices.Clone(index.Key), + Unique: index.Unique, + } + } + + return Settings{ + Indexes: indexes, + } +} + +// marshal returns [*types.Document] for settings. +func (s Settings) marshal() *types.Document { + indexes := types.MakeArray(len(s.Indexes)) + + for _, index := range s.Indexes { + key := types.MakeDocument(len(index.Key)) + + // The format of the index key storing was defined in the early versions of FerretDB, + // it's kept for backward compatibility. + for _, pair := range index.Key { + order := int32(1) // order is set as int32 to be sjson-marshaled correctly + + if pair.Descending { + order = -1 + } + + key.Set(pair.Field, order) + } + + indexes.Append(must.NotFail(types.NewDocument( + "pgindex", index.PgName, + "name", index.Name, + "key", key, + "unique", index.Unique, + ))) + } + + return must.NotFail(types.NewDocument( + "indexes", indexes, + )) +} + +// unmarshal sets settings from [*types.Document]. +func (s *Settings) unmarshal(doc *types.Document) error { + indexes := must.NotFail(doc.Get("indexes")).(*types.Array) + + s.Indexes = make([]IndexInfo, indexes.Len()) + + iter := indexes.Iterator() + defer iter.Close() + + for { + i, v, err := iter.Next() + if errors.Is(err, iterator.ErrIteratorDone) { + break + } + + if err != nil { + return lazyerrors.Error(err) + } + + doc := v.(*types.Document) + + keyDoc := must.NotFail(doc.Get("key")).(*types.Document) + keyIter := keyDoc.Iterator() + key := make([]IndexKeyPair, keyDoc.Len()) + + defer keyIter.Close() + + for j := 0; ; j++ { + field, order, err := keyIter.Next() + if errors.Is(err, iterator.ErrIteratorDone) { + break + } + + if err != nil { + return lazyerrors.Error(err) + } + + descending := false + if order.(int32) == -1 { + descending = true + } + + key[j] = IndexKeyPair{ + Field: field, + Descending: descending, + } + } + + s.Indexes[i] = IndexInfo{ + Name: must.NotFail(doc.Get("name")).(string), + PgName: must.NotFail(doc.Get("pgindex")).(string), + Key: key, + Unique: must.NotFail(doc.Get("unique")).(bool), + } + } + + return nil +} diff --git a/internal/backends/sqlite/metadata/metadata.go b/internal/backends/sqlite/metadata/metadata.go index 89ffe3e4bd34..2f78f53a40f7 100644 --- a/internal/backends/sqlite/metadata/metadata.go +++ b/internal/backends/sqlite/metadata/metadata.go @@ -15,16 +15,6 @@ // Package metadata provides access to databases and collections information. package metadata -import ( - "database/sql" - "database/sql/driver" - "encoding/json" - - "golang.org/x/exp/slices" - - "github.com/FerretDB/FerretDB/internal/util/lazyerrors" -) - // Collection will probably have a method for getting column name / SQLite path expression for the given document field // once we implement field extraction. // IDColumn probably should go away. @@ -60,76 +50,3 @@ func (c *Collection) deepCopy() *Collection { Settings: c.Settings.deepCopy(), } } - -// Settings represents collection settings. -type Settings struct { - Indexes []IndexInfo `json:"indexes"` -} - -// deepCopy returns a deep copy. -func (s Settings) deepCopy() Settings { - indexes := make([]IndexInfo, len(s.Indexes)) - - for i, index := range s.Indexes { - indexes[i] = IndexInfo{ - Name: index.Name, - Key: slices.Clone(index.Key), - Unique: index.Unique, - } - } - - return Settings{ - Indexes: indexes, - } -} - -// Value implements driver.Valuer interface. -func (s Settings) Value() (driver.Value, error) { - res, err := json.Marshal(s) - if err != nil { - return nil, lazyerrors.Error(err) - } - - return string(res), nil -} - -// Scan implements sql.Scanner interface. -func (s *Settings) Scan(src any) error { - var err error - - switch src := src.(type) { - case nil: - *s = Settings{} - case []byte: - err = json.Unmarshal(src, s) - case string: - err = json.Unmarshal([]byte(src), s) - default: - panic("can't scan collection settings") - } - - if err != nil { - return lazyerrors.Error(err) - } - - return nil -} - -// IndexInfo represents information about a single index. -type IndexInfo struct { - Name string `json:"name"` - Key []IndexKeyPair `json:"key"` - Unique bool `json:"unique"` -} - -// IndexKeyPair consists of a field name and a sort order that are part of the index. -type IndexKeyPair struct { - Field string `json:"field"` - Descending bool `json:"descending"` -} - -// check interfaces -var ( - _ driver.Valuer = Settings{} - _ sql.Scanner = (*Settings)(nil) -) diff --git a/internal/backends/sqlite/metadata/settings.go b/internal/backends/sqlite/metadata/settings.go new file mode 100644 index 000000000000..94867fcf8c17 --- /dev/null +++ b/internal/backends/sqlite/metadata/settings.go @@ -0,0 +1,98 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metadata + +import ( + "database/sql" + "database/sql/driver" + "encoding/json" + + "golang.org/x/exp/slices" + + "github.com/FerretDB/FerretDB/internal/util/lazyerrors" +) + +// Settings represents collection settings. +type Settings struct { + Indexes []IndexInfo `json:"indexes"` +} + +// IndexInfo represents information about a single index. +type IndexInfo struct { + Name string `json:"name"` + Key []IndexKeyPair `json:"key"` + Unique bool `json:"unique"` +} + +// IndexKeyPair consists of a field name and a sort order that are part of the index. +type IndexKeyPair struct { + Field string `json:"field"` + Descending bool `json:"descending"` +} + +// deepCopy returns a deep copy. +func (s Settings) deepCopy() Settings { + indexes := make([]IndexInfo, len(s.Indexes)) + + for i, index := range s.Indexes { + indexes[i] = IndexInfo{ + Name: index.Name, + Key: slices.Clone(index.Key), + Unique: index.Unique, + } + } + + return Settings{ + Indexes: indexes, + } +} + +// Value implements driver.Valuer interface. +func (s Settings) Value() (driver.Value, error) { + res, err := json.Marshal(s) + if err != nil { + return nil, lazyerrors.Error(err) + } + + return string(res), nil +} + +// Scan implements sql.Scanner interface. +func (s *Settings) Scan(src any) error { + var err error + + switch src := src.(type) { + case nil: + *s = Settings{} + case []byte: + err = json.Unmarshal(src, s) + case string: + err = json.Unmarshal([]byte(src), s) + default: + panic("can't scan collection settings") + } + + if err != nil { + return lazyerrors.Error(err) + } + + return nil +} + +// check interfaces +var ( + _ driver.Valuer = Settings{} + _ sql.Scanner = (*Settings)(nil) +) From 402e731aa92e7b3d1c794fdc26c3d3075d8d3e01 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Mon, 2 Oct 2023 18:28:32 +0400 Subject: [PATCH 30/39] Refactor --- .../backends/postgresql/metadata/indexes.go | 138 ++++++++++++++++ .../backends/postgresql/metadata/metadata.go | 26 +-- .../backends/postgresql/metadata/registry.go | 14 +- .../postgresql/metadata/registry_test.go | 26 +-- .../backends/postgresql/metadata/settings.go | 154 ------------------ 5 files changed, 172 insertions(+), 186 deletions(-) create mode 100644 internal/backends/postgresql/metadata/indexes.go delete mode 100644 internal/backends/postgresql/metadata/settings.go diff --git a/internal/backends/postgresql/metadata/indexes.go b/internal/backends/postgresql/metadata/indexes.go new file mode 100644 index 000000000000..287473880752 --- /dev/null +++ b/internal/backends/postgresql/metadata/indexes.go @@ -0,0 +1,138 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metadata + +import ( + "errors" + + "golang.org/x/exp/slices" + + "github.com/FerretDB/FerretDB/internal/types" + "github.com/FerretDB/FerretDB/internal/util/iterator" + "github.com/FerretDB/FerretDB/internal/util/lazyerrors" + "github.com/FerretDB/FerretDB/internal/util/must" +) + +// Indexes represents information about all indexes in a collection. +type Indexes []IndexInfo + +// IndexInfo represents information about a single index. +type IndexInfo struct { + Name string + PgIndex string + Key []IndexKeyPair + Unique bool +} + +// IndexKeyPair consists of a field name and a sort order that are part of the index. +type IndexKeyPair struct { + Field string + Descending bool +} + +// deepCopy returns a deep copy. +func (indexes Indexes) deepCopy() Indexes { + res := make(Indexes, len(indexes)) + + for i, index := range indexes { + res[i] = IndexInfo{ + Name: index.Name, + PgIndex: index.PgIndex, + Key: slices.Clone(index.Key), + Unique: index.Unique, + } + } + + return res +} + +// marshal returns [*types.Array] for indexes. +func (indexes Indexes) marshal() *types.Array { + res := types.MakeArray(len(indexes)) + + for _, index := range indexes { + key := types.MakeDocument(len(index.Key)) + + for _, pair := range index.Key { + order := int32(1) + if pair.Descending { + order = int32(-1) + } + + key.Set(pair.Field, order) + } + + res.Append(must.NotFail(types.NewDocument( + "pgindex", index.PgIndex, + "name", index.Name, + "key", key, + "unique", index.Unique, + ))) + } + + return res +} + +// unmarshal sets indexes from [*types.Array]. +func (s *Indexes) unmarshal(a *types.Array) error { + res := make(Indexes, a.Len()) + + iter := a.Iterator() + defer iter.Close() + + for { + i, v, err := iter.Next() + if errors.Is(err, iterator.ErrIteratorDone) { + break + } + + if err != nil { + return lazyerrors.Error(err) + } + + index := v.(*types.Document) + + keyDoc := must.NotFail(index.Get("key")).(*types.Document) + fields := keyDoc.Keys() + orders := keyDoc.Values() + key := make([]IndexKeyPair, keyDoc.Len()) + + for j, f := range fields { + descending := false + if orders[j].(int32) == -1 { + descending = true + } + + key[j] = IndexKeyPair{ + Field: f, + Descending: descending, + } + } + + // it was possible for it to be null in pgdb + v, _ = index.Get("unique") + unique, _ := v.(bool) + + res[i] = IndexInfo{ + Name: must.NotFail(index.Get("name")).(string), + PgIndex: must.NotFail(index.Get("pgindex")).(string), + Key: key, + Unique: unique, + } + } + + *s = res + return nil +} diff --git a/internal/backends/postgresql/metadata/metadata.go b/internal/backends/postgresql/metadata/metadata.go index e7ff3472f396..e7e5ca6240db 100644 --- a/internal/backends/postgresql/metadata/metadata.go +++ b/internal/backends/postgresql/metadata/metadata.go @@ -40,7 +40,7 @@ const ( type Collection struct { Name string TableName string - Settings Settings + Indexes Indexes } // deepCopy returns a deep copy. @@ -52,7 +52,7 @@ func (c *Collection) deepCopy() *Collection { return &Collection{ Name: c.Name, TableName: c.TableName, - Settings: c.Settings.deepCopy(), + Indexes: c.Indexes.deepCopy(), } } @@ -70,6 +70,7 @@ func (c Collection) Value() (driver.Value, error) { func (c *Collection) Scan(src any) error { var doc *types.Document var err error + switch src := src.(type) { case nil: *c = Collection{} @@ -98,7 +99,7 @@ func (c *Collection) marshal() *types.Document { return must.NotFail(types.NewDocument( "_id", c.Name, "table", c.TableName, - "settings", c.Settings.marshal(), + "indexes", c.Indexes.marshal(), )) } @@ -106,27 +107,28 @@ func (c *Collection) marshal() *types.Document { func (c *Collection) unmarshal(doc *types.Document) error { v, _ := doc.Get("_id") c.Name, _ = v.(string) + if c.Name == "" { return lazyerrors.New("collection name is empty") } v, _ = doc.Get("table") c.TableName, _ = v.(string) + if c.TableName == "" { return lazyerrors.New("table name is empty") } - v, _ = doc.Get("settings") - s, _ := v.(*types.Document) - if s == nil { - return lazyerrors.New("settings are empty") - } + v, _ = doc.Get("indexes") + i, _ := v.(*types.Array) - c.Settings.unmarshal(doc) + if i == nil { + return lazyerrors.New("indexes are empty") + } - var settings Settings - must.NoError(settings.unmarshal(must.NotFail(doc.Get("settings")).(*types.Document))) - c.Settings = settings + if err := c.Indexes.unmarshal(i); err != nil { + return lazyerrors.Error(err) + } return nil } diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 44d6ba64ac0e..3c9eb976189f 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -759,7 +759,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c created := make([]string, 0, len(indexes)) for _, index := range indexes { - if slices.ContainsFunc(c.Settings.Indexes, func(i IndexInfo) bool { return index.Name == i.Name }) { + if slices.ContainsFunc(c.Indexes, func(i IndexInfo) bool { return index.Name == i.Name }) { continue } @@ -772,7 +772,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c tableNamePart = tableNamePart[:tableNamePartMax] } - index.PgName = fmt.Sprintf("%s_%s_idx", tableNamePart, uuidPart) + index.PgIndex = fmt.Sprintf("%s_%s_idx", tableNamePart, uuidPart) q := "CREATE " @@ -802,7 +802,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c q = fmt.Sprintf( q, - pgx.Identifier{index.PgName}.Sanitize(), + pgx.Identifier{index.PgIndex}.Sanitize(), pgx.Identifier{dbName, c.TableName}.Sanitize(), strings.Join(columns, ", "), ) @@ -813,7 +813,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c } created = append(created, index.Name) - c.Settings.Indexes = append(c.Settings.Indexes, index) + c.Indexes = append(c.Indexes, index) } b, err := sjson.Marshal(c.marshal()) @@ -879,17 +879,17 @@ func (r *Registry) indexesDrop(ctx context.Context, p *pgxpool.Pool, dbName, col } for _, name := range indexNames { - i := slices.IndexFunc(c.Settings.Indexes, func(i IndexInfo) bool { return name == i.Name }) + i := slices.IndexFunc(c.Indexes, func(i IndexInfo) bool { return name == i.Name }) if i < 0 { continue } - q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Settings.Indexes[i].PgName}.Sanitize()) + q := fmt.Sprintf("DROP INDEX %s", pgx.Identifier{dbName, c.Indexes[i].PgIndex}.Sanitize()) if _, err := p.Exec(ctx, q); err != nil { return lazyerrors.Error(err) } - c.Settings.Indexes = slices.Delete(c.Settings.Indexes, i, i+1) + c.Indexes = slices.Delete(c.Indexes, i, i+1) } b, err := sjson.Marshal(c.marshal()) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 0543a4f88544..213d6039d7bd 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -441,7 +441,7 @@ func TestRenameCollection(t *testing.T) { expected := &Collection{ Name: newCollectionName, TableName: oldCollection.TableName, - Settings: oldCollection.Settings, + Indexes: oldCollection.Indexes, } actual, err := r.CollectionGet(ctx, dbName, newCollectionName) @@ -508,11 +508,11 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) t.Run("NonUniqueIndex", func(t *testing.T) { - i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { return ii.Name == "index_non_unique" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].PgName + tableIndexName := collection.Indexes[i].PgIndex var sql string err = db.QueryRow( @@ -530,11 +530,11 @@ func TestIndexesCreateDrop(t *testing.T) { }) t.Run("UniqueIndex", func(t *testing.T) { - i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { return ii.Name == "index_unique" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].PgName + tableIndexName := collection.Indexes[i].PgIndex var sql string err = db.QueryRow( @@ -552,11 +552,11 @@ func TestIndexesCreateDrop(t *testing.T) { }) t.Run("NestedFields", func(t *testing.T) { - i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { return ii.Name == "nested_fields" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].PgName + tableIndexName := collection.Indexes[i].PgIndex var sql string err = db.QueryRow( @@ -575,11 +575,11 @@ func TestIndexesCreateDrop(t *testing.T) { }) t.Run("DefaultIndex", func(t *testing.T) { - i := slices.IndexFunc(collection.Settings.Indexes, func(ii IndexInfo) bool { + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { return ii.Name == "_id_" }) require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Settings.Indexes[i].PgName + tableIndexName := collection.Indexes[i].PgIndex var sql string err = db.QueryRow( @@ -604,9 +604,9 @@ func TestIndexesCreateDrop(t *testing.T) { refreshedCollection, err = r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - require.Equal(t, 4, len(refreshedCollection.Settings.Indexes)) + require.Equal(t, 4, len(refreshedCollection.Indexes)) - for _, index := range refreshedCollection.Settings.Indexes { + for _, index := range refreshedCollection.Indexes { switch { case index.Name == "_id_": assert.Equal(t, 1, len(index.Key)) @@ -640,9 +640,9 @@ func TestIndexesCreateDrop(t *testing.T) { collection, err = r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - require.Equal(t, 2, len(collection.Settings.Indexes)) + require.Equal(t, 2, len(collection.Indexes)) - for _, index := range collection.Settings.Indexes { + for _, index := range collection.Indexes { switch index.Name { case "_id_": assert.Equal(t, 1, len(index.Key)) diff --git a/internal/backends/postgresql/metadata/settings.go b/internal/backends/postgresql/metadata/settings.go deleted file mode 100644 index aa6b00dca30f..000000000000 --- a/internal/backends/postgresql/metadata/settings.go +++ /dev/null @@ -1,154 +0,0 @@ -// Copyright 2021 FerretDB Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package metadata - -import ( - "errors" - - "golang.org/x/exp/slices" - - "github.com/FerretDB/FerretDB/internal/types" - "github.com/FerretDB/FerretDB/internal/util/iterator" - "github.com/FerretDB/FerretDB/internal/util/lazyerrors" - "github.com/FerretDB/FerretDB/internal/util/must" -) - -// Settings represents collection settings. -type Settings struct { - Indexes []IndexInfo -} - -// IndexInfo represents information about a single index. -type IndexInfo struct { - Name string - PgName string - Key []IndexKeyPair - Unique bool -} - -// IndexKeyPair consists of a field name and a sort order that are part of the index. -type IndexKeyPair struct { - Field string - Descending bool -} - -// deepCopy returns a deep copy. -func (s Settings) deepCopy() Settings { - indexes := make([]IndexInfo, len(s.Indexes)) - - for i, index := range s.Indexes { - indexes[i] = IndexInfo{ - Name: index.Name, - PgName: index.PgName, - Key: slices.Clone(index.Key), - Unique: index.Unique, - } - } - - return Settings{ - Indexes: indexes, - } -} - -// marshal returns [*types.Document] for settings. -func (s Settings) marshal() *types.Document { - indexes := types.MakeArray(len(s.Indexes)) - - for _, index := range s.Indexes { - key := types.MakeDocument(len(index.Key)) - - // The format of the index key storing was defined in the early versions of FerretDB, - // it's kept for backward compatibility. - for _, pair := range index.Key { - order := int32(1) // order is set as int32 to be sjson-marshaled correctly - - if pair.Descending { - order = -1 - } - - key.Set(pair.Field, order) - } - - indexes.Append(must.NotFail(types.NewDocument( - "pgindex", index.PgName, - "name", index.Name, - "key", key, - "unique", index.Unique, - ))) - } - - return must.NotFail(types.NewDocument( - "indexes", indexes, - )) -} - -// unmarshal sets settings from [*types.Document]. -func (s *Settings) unmarshal(doc *types.Document) error { - indexes := must.NotFail(doc.Get("indexes")).(*types.Array) - - s.Indexes = make([]IndexInfo, indexes.Len()) - - iter := indexes.Iterator() - defer iter.Close() - - for { - i, v, err := iter.Next() - if errors.Is(err, iterator.ErrIteratorDone) { - break - } - - if err != nil { - return lazyerrors.Error(err) - } - - doc := v.(*types.Document) - - keyDoc := must.NotFail(doc.Get("key")).(*types.Document) - keyIter := keyDoc.Iterator() - key := make([]IndexKeyPair, keyDoc.Len()) - - defer keyIter.Close() - - for j := 0; ; j++ { - field, order, err := keyIter.Next() - if errors.Is(err, iterator.ErrIteratorDone) { - break - } - - if err != nil { - return lazyerrors.Error(err) - } - - descending := false - if order.(int32) == -1 { - descending = true - } - - key[j] = IndexKeyPair{ - Field: field, - Descending: descending, - } - } - - s.Indexes[i] = IndexInfo{ - Name: must.NotFail(doc.Get("name")).(string), - PgName: must.NotFail(doc.Get("pgindex")).(string), - Key: key, - Unique: must.NotFail(doc.Get("unique")).(bool), - } - } - - return nil -} From 3fc91567e4bed58dce50e7834ce5c17f31d213d8 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Mon, 2 Oct 2023 18:38:10 +0400 Subject: [PATCH 31/39] Cleanup --- .../backends/postgresql/metadata/indexes.go | 1 + .../postgresql/metadata/registry_test.go | 101 ++++++------------ 2 files changed, 31 insertions(+), 71 deletions(-) diff --git a/internal/backends/postgresql/metadata/indexes.go b/internal/backends/postgresql/metadata/indexes.go index 287473880752..246c3e10ce34 100644 --- a/internal/backends/postgresql/metadata/indexes.go +++ b/internal/backends/postgresql/metadata/indexes.go @@ -134,5 +134,6 @@ func (s *Indexes) unmarshal(a *types.Array) error { } *s = res + return nil } diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 213d6039d7bd..5598fc592dd7 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -17,7 +17,6 @@ package metadata import ( "context" "fmt" - "strings" "sync/atomic" "testing" @@ -164,34 +163,6 @@ func TestCreateDrop(t *testing.T) { testCollection(t, ctx, r, db, dbName, collectionName) } -func TestCreateLongCollectionName(t *testing.T) { - if testing.Short() { - t.Skip("skipping in -short mode") - } - - t.Parallel() - - connInfo := conninfo.New() - ctx := conninfo.Ctx(testutil.Ctx(t), connInfo) - - r, _, dbName := createDatabase(t, ctx) - - collectionName := strings.Repeat("a", 63) - created, err := r.CollectionCreate(ctx, dbName, collectionName) - require.NoError(t, err) - require.True(t, created) - - collectionName = strings.Repeat("a", 63) - created, err = r.CollectionCreate(ctx, dbName, collectionName) - require.NoError(t, err) - require.False(t, created) - - collectionName = strings.Repeat("a", 64) - created, err = r.CollectionCreate(ctx, dbName, collectionName) - require.NoError(t, err) - require.True(t, created) -} - func TestCreateDropStress(t *testing.T) { if testing.Short() { t.Skip("skipping in -short mode") @@ -463,43 +434,31 @@ func TestIndexesCreateDrop(t *testing.T) { r, db, dbName := createDatabase(t, ctx) collectionName := testutil.CollectionName(t) - toCreate := []IndexInfo{ - { - Name: "index_non_unique", - Key: []IndexKeyPair{ - { - Field: "f1", - Descending: false, - }, - { - Field: "f2", - Descending: true, - }, - }, - }, - { - Name: "index_unique", - Key: []IndexKeyPair{ - { - Field: "foo", - Descending: false, - }, - }, - Unique: true, - }, - { - Name: "nested_fields", - Key: []IndexKeyPair{ - { - Field: "foo.bar", - }, - { - Field: "foo.baz", - Descending: true, - }, - }, - }, - } + toCreate := []IndexInfo{{ + Name: "index_non_unique", + Key: []IndexKeyPair{{ + Field: "f1", + Descending: false, + }, { + Field: "f2", + Descending: true, + }}, + }, { + Name: "index_unique", + Key: []IndexKeyPair{{ + Field: "foo", + Descending: false, + }}, + Unique: true, + }, { + Name: "nested_fields", + Key: []IndexKeyPair{{ + Field: "foo.bar", + }, { + Field: "foo.baz", + Descending: true, + }}, + }} err := r.IndexesCreate(ctx, dbName, collectionName, toCreate) require.NoError(t, err) @@ -607,14 +566,14 @@ func TestIndexesCreateDrop(t *testing.T) { require.Equal(t, 4, len(refreshedCollection.Indexes)) for _, index := range refreshedCollection.Indexes { - switch { - case index.Name == "_id_": + switch index.Name { + case "_id_": assert.Equal(t, 1, len(index.Key)) - case index.Name == "index_non_unique": + case "index_non_unique": assert.Equal(t, 2, len(index.Key)) - case index.Name == "index_unique": + case "index_unique": assert.Equal(t, 1, len(index.Key)) - case index.Name == "nested_fields": + case "nested_fields": assert.Equal(t, 2, len(index.Key)) default: t.Errorf("unexpected index: %s", index.Name) From 9819817b4ba241be9db190bc65086e1a0fbf1534 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Mon, 2 Oct 2023 18:51:03 +0400 Subject: [PATCH 32/39] Remove tests for library functions --- .../backends/postgresql/collection_test.go | 67 -------- internal/backends/postgresql/dummy_test.go | 23 +++ .../postgresql/metadata/pool/dummy_test.go | 23 +++ .../metadata/pool/transaction_test.go | 155 ------------------ internal/backends/sqlite/collection_test.go | 31 ---- 5 files changed, 46 insertions(+), 253 deletions(-) delete mode 100644 internal/backends/postgresql/collection_test.go create mode 100644 internal/backends/postgresql/dummy_test.go create mode 100644 internal/backends/postgresql/metadata/pool/dummy_test.go delete mode 100644 internal/backends/postgresql/metadata/pool/transaction_test.go diff --git a/internal/backends/postgresql/collection_test.go b/internal/backends/postgresql/collection_test.go deleted file mode 100644 index d9bb62671a20..000000000000 --- a/internal/backends/postgresql/collection_test.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2021 FerretDB Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package postgresql - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/FerretDB/FerretDB/internal/backends" - "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" - "github.com/FerretDB/FerretDB/internal/types" - "github.com/FerretDB/FerretDB/internal/util/state" - "github.com/FerretDB/FerretDB/internal/util/testutil" -) - -func TestInsertAll(t *testing.T) { - if testing.Short() { - t.Skip("skipping in -short mode") - } - - sp, err := state.NewProvider("") - require.NoError(t, err) - - params := NewBackendParams{ - URI: "postgres://username:password@127.0.0.1:5432/ferretdb?pool_min_conns=1", - L: testutil.Logger(t), - P: sp, - } - b, err := NewBackend(¶ms) - require.NoError(t, err) - - t.Cleanup(b.Close) - - db, err := b.Database(testutil.DatabaseName(t)) - require.NoError(t, err) - - c, err := db.Collection(testutil.CollectionName(t)) - require.NoError(t, err) - - ctx := conninfo.Ctx(testutil.Ctx(t), conninfo.New()) - - doc, err := types.NewDocument("_id", types.NewObjectID()) - require.NoError(t, err) - - _, err = c.InsertAll(ctx, &backends.InsertAllParams{ - Docs: []*types.Document{doc}, - }) - require.NoError(t, err) - // TODO https://github.com/FerretDB/FerretDB/issues/3375 - //_, err = c.InsertAll(ctx, &backends.InsertAllParams{ - // Docs: []*types.Document{doc}, - //}) - //require.True(t, backends.ErrorCodeIs(err, backends.ErrorCodeInsertDuplicateID)) -} diff --git a/internal/backends/postgresql/dummy_test.go b/internal/backends/postgresql/dummy_test.go new file mode 100644 index 000000000000..5c750a7cfa31 --- /dev/null +++ b/internal/backends/postgresql/dummy_test.go @@ -0,0 +1,23 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package postgresql + +import ( + "testing" +) + +func TestDummy(t *testing.T) { + // we need at least one test per package to correctly calculate coverage +} diff --git a/internal/backends/postgresql/metadata/pool/dummy_test.go b/internal/backends/postgresql/metadata/pool/dummy_test.go new file mode 100644 index 000000000000..cba87bfbf6ad --- /dev/null +++ b/internal/backends/postgresql/metadata/pool/dummy_test.go @@ -0,0 +1,23 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pool + +import ( + "testing" +) + +func TestDummy(t *testing.T) { + // we need at least one test per package to correctly calculate coverage +} diff --git a/internal/backends/postgresql/metadata/pool/transaction_test.go b/internal/backends/postgresql/metadata/pool/transaction_test.go deleted file mode 100644 index 55ac36935f38..000000000000 --- a/internal/backends/postgresql/metadata/pool/transaction_test.go +++ /dev/null @@ -1,155 +0,0 @@ -// Copyright 2021 FerretDB Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package pool - -import ( - "context" - "errors" - "fmt" - "testing" - - "github.com/jackc/pgx/v5" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" - "github.com/FerretDB/FerretDB/internal/util/state" - "github.com/FerretDB/FerretDB/internal/util/testutil" -) - -func TestInTransaction(t *testing.T) { - if testing.Short() { - t.Skip("skipping in -short mode") - } - - t.Parallel() - - ctx := conninfo.Ctx(testutil.Ctx(t), conninfo.New()) - username, password := conninfo.Get(ctx).Auth() - - sp, err := state.NewProvider("") - require.NoError(t, err) - - u := "postgres://username:password@127.0.0.1:5432/ferretdb?pool_min_conns=1" - - pp, err := New(u, testutil.Logger(t), sp) - require.NoError(t, err) - - t.Cleanup(pp.Close) - - p, err := pp.Get(username, password) - require.NoError(t, err) - - t.Cleanup(p.Close) - - tableName := t.Name() - _, err = p.Exec(ctx, fmt.Sprintf(`DROP TABLE IF EXISTS %[1]s; CREATE TABLE %[1]s(s TEXT);`, tableName)) - require.NoError(t, err) - - t.Cleanup(func() { - _, err = p.Exec(ctx, fmt.Sprintf(`DROP TABLE %s`, tableName)) - require.NoError(t, err) - }) - - t.Run("Commit", func(t *testing.T) { - t.Parallel() - - ctx := conninfo.Ctx(testutil.Ctx(t), conninfo.New()) // create new instance of ctx to avoid using canceled ctx - err := err // avoid data race - - v := testutil.CollectionName(t) - err = InTransaction(ctx, p, func(tx pgx.Tx) error { - _, err = tx.Exec(ctx, fmt.Sprintf(`INSERT INTO %s(s) VALUES ($1)`, tableName), v) - require.NoError(t, err) - return nil - }) - require.NoError(t, err) - - var res string - err = p.QueryRow(ctx, fmt.Sprintf(`SELECT s FROM %s WHERE s = $1`, tableName), v).Scan(&res) - require.NoError(t, err) - require.Equal(t, v, res) - }) - - t.Run("Rollback", func(t *testing.T) { - t.Parallel() - - ctx := conninfo.Ctx(testutil.Ctx(t), conninfo.New()) // create new instance of ctx to avoid using canceled ctx - err := err // avoid data race - - v := testutil.CollectionName(t) - err = InTransaction(ctx, p, func(tx pgx.Tx) error { - _, err = tx.Exec(ctx, fmt.Sprintf(`INSERT INTO %s(s) VALUES ($1)`, tableName), v) - require.NoError(t, err) - return errors.New("boom") - }) - require.Error(t, err) - - var res string - err = p.QueryRow(ctx, fmt.Sprintf(`SELECT s FROM %s WHERE s = $1`, tableName), v).Scan(&res) - require.Equal(t, pgx.ErrNoRows, err) - require.Empty(t, res) - }) - - t.Run("ContextCancelRollback", func(t *testing.T) { - t.Parallel() - - ctx := conninfo.Ctx(testutil.Ctx(t), conninfo.New()) // create new instance of ctx to avoid using canceled ctx - err := err // avoid data race - - var cancel func() - ctx, cancel = context.WithCancel(ctx) - - v := testutil.CollectionName(t) - err = InTransaction(ctx, p, func(tx pgx.Tx) error { - _, err = tx.Exec(ctx, fmt.Sprintf(`INSERT INTO %s(s) VALUES ($1)`, tableName), v) - require.NoError(t, err) - - cancel() - - return nil - }) - require.Error(t, err) - - var res string - err = p.QueryRow(context.WithoutCancel(ctx), fmt.Sprintf(`SELECT s FROM %s WHERE s = $1`, tableName), v).Scan(&res) - require.Equal(t, pgx.ErrNoRows, err) - require.Empty(t, res) - }) - - t.Run("Panic", func(t *testing.T) { - t.Parallel() - - ctx := conninfo.Ctx(testutil.Ctx(t), conninfo.New()) // create new instance of ctx to avoid using canceled ctx - err := err // avoid data race - - v := testutil.CollectionName(t) - assert.Panics(t, func() { - err = InTransaction(ctx, p, func(tx pgx.Tx) error { - _, err = tx.Exec(ctx, fmt.Sprintf(`INSERT INTO %s(s) VALUES ($1)`, tableName), v) - require.NoError(t, err) - - //nolint:vet // need it for testing - panic(nil) - }) - require.Equal(t, pgx.ErrNoRows, err) - }) - - var res string - err = p.QueryRow(ctx, fmt.Sprintf(`SELECT s FROM %s WHERE s = $1`, tableName), v).Scan(&res) - require.Equal(t, pgx.ErrNoRows, err) - require.Empty(t, res) - }) -} diff --git a/internal/backends/sqlite/collection_test.go b/internal/backends/sqlite/collection_test.go index 5c40e0b5f47a..90296a31dbc7 100644 --- a/internal/backends/sqlite/collection_test.go +++ b/internal/backends/sqlite/collection_test.go @@ -26,37 +26,6 @@ import ( "github.com/FerretDB/FerretDB/internal/util/testutil" ) -func TestInsert(t *testing.T) { - sp, err := state.NewProvider("") - require.NoError(t, err) - - b, err := NewBackend(&NewBackendParams{URI: "file:./?mode=memory", L: testutil.Logger(t), P: sp}) - require.NoError(t, err) - - defer b.Close() - - db, err := b.Database(testutil.DatabaseName(t)) - require.NoError(t, err) - - c, err := db.Collection(testutil.CollectionName(t)) - require.NoError(t, err) - - ctx := testutil.Ctx(t) - - doc, err := types.NewDocument("_id", types.NewObjectID()) - require.NoError(t, err) - - _, err = c.InsertAll(ctx, &backends.InsertAllParams{ - Docs: []*types.Document{doc}, - }) - require.NoError(t, err) - - _, err = c.InsertAll(ctx, &backends.InsertAllParams{ - Docs: []*types.Document{doc}, - }) - require.True(t, backends.ErrorCodeIs(err, backends.ErrorCodeInsertDuplicateID)) -} - func TestCollectionStats(t *testing.T) { t.Parallel() ctx := testutil.Ctx(t) From 47de006bb3c45b800b9b627b064a02cf8059b461 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 13:03:11 +0200 Subject: [PATCH 33/39] version without uuid --- .../postgresql/metadata/pool/dummy_test.go | 4 +- .../backends/postgresql/metadata/registry.go | 40 ++++++++++++++++--- .../postgresql/metadata/registry_test.go | 8 ++-- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/internal/backends/postgresql/metadata/pool/dummy_test.go b/internal/backends/postgresql/metadata/pool/dummy_test.go index cba87bfbf6ad..6d845abd4831 100644 --- a/internal/backends/postgresql/metadata/pool/dummy_test.go +++ b/internal/backends/postgresql/metadata/pool/dummy_test.go @@ -14,9 +14,7 @@ package pool -import ( - "testing" -) +import "testing" func TestDummy(t *testing.T) { // we need at least one test per package to correctly calculate coverage diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 3c9eb976189f..fa62633dd472 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -24,7 +24,6 @@ import ( "strings" "sync" - "github.com/google/uuid" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/prometheus/client_golang/prometheus" @@ -763,16 +762,47 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c continue } - // indexes must be unique across the whole database, so we add a uuid to the index name - uuidPart := uuid.NewString() tableNamePart := c.TableName - tableNamePartMax := maxIndexNameLength - len(uuidPart) - 5 // 5 is for _ and _idx + tableNamePartMax := maxIndexNameLength/2 - 1 // 1 for the separator between table name and index name if len(tableNamePart) > tableNamePartMax { tableNamePart = tableNamePart[:tableNamePartMax] } - index.PgIndex = fmt.Sprintf("%s_%s_idx", tableNamePart, uuidPart) + indexNamePart := specialCharacters.ReplaceAllString(strings.ToLower(index.Name), "_") + + h := fnv.New32a() + must.NotFail(h.Write([]byte(collectionName))) + s := h.Sum32() + + var pgIndexName string + + for { + suffixHash := fmt.Sprintf("_%08x_idx", s) + if l := maxIndexNameLength/2 - len(suffixHash); len(indexNamePart) > l { + indexNamePart = indexNamePart[:l] + } + + pgIndexName = fmt.Sprintf("%s%s", tableNamePart, indexNamePart) + + // indexes must be unique across the whole database, so we check for duplicates for all collections + var duplicate bool + + for _, coll := range db { + if slices.ContainsFunc(coll.Indexes, func(i IndexInfo) bool { return pgIndexName == i.PgIndex }) { + duplicate = true + s++ + + break + } + } + + if !duplicate { + break + } + } + + index.PgIndex = pgIndexName q := "CREATE " diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 5598fc592dd7..d927cc885050 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -482,7 +482,7 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE INDEX %q ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, + `CREATE INDEX %s ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) @@ -504,7 +504,7 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE UNIQUE INDEX %q ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) @@ -526,7 +526,7 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE INDEX %q ON %q.%s USING btree`+ + `CREATE INDEX %s ON %q.%s USING btree`+ ` ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, tableIndexName, dbName, collection.TableName, ) @@ -549,7 +549,7 @@ func TestIndexesCreateDrop(t *testing.T) { require.NoError(t, err) expected := fmt.Sprintf( - `CREATE UNIQUE INDEX %q ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, tableIndexName, dbName, collection.TableName, ) require.Equal(t, expected, sql) From 11958aec1525eb29c1f51e422109412d6a6afd25 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 14:06:39 +0200 Subject: [PATCH 34/39] simpler way to check for existing indexes --- internal/backends/{doc.go => backends.go} | 3 + internal/backends/postgresql/dummy_test.go | 4 +- .../backends/postgresql/metadata/registry.go | 25 +-- .../postgresql/metadata/registry_test.go | 170 +++++++++--------- 4 files changed, 108 insertions(+), 94 deletions(-) rename internal/backends/{doc.go => backends.go} (95%) diff --git a/internal/backends/doc.go b/internal/backends/backends.go similarity index 95% rename from internal/backends/doc.go rename to internal/backends/backends.go index 48560fcbbc6b..700dbacbc252 100644 --- a/internal/backends/doc.go +++ b/internal/backends/backends.go @@ -43,5 +43,8 @@ // But there are some common tests for all backends that check corner cases in contracts. // They also test that all backends adjusted to contract changes. // +// Some backends may have their own tests. +// +// Both kinds of tests could be removed over time as they are replaced by integration tests. // Prefer integration tests when possible. package backends diff --git a/internal/backends/postgresql/dummy_test.go b/internal/backends/postgresql/dummy_test.go index 5c750a7cfa31..ab1be0278a87 100644 --- a/internal/backends/postgresql/dummy_test.go +++ b/internal/backends/postgresql/dummy_test.go @@ -14,9 +14,7 @@ package postgresql -import ( - "testing" -) +import "testing" func TestDummy(t *testing.T) { // we need at least one test per package to correctly calculate coverage diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index fa62633dd472..24eed2bf3d59 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -755,10 +755,20 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c panic("collection does not exist") } + allIndexes := make(map[string]string, len(db)) + allPgIndexes := make(map[string]string, len(db)) + + for _, coll := range db { + for _, index := range coll.Indexes { + allIndexes[index.Name] = coll.Name + allPgIndexes[index.PgIndex] = coll.Name + } + } + created := make([]string, 0, len(indexes)) for _, index := range indexes { - if slices.ContainsFunc(c.Indexes, func(i IndexInfo) bool { return index.Name == i.Name }) { + if coll, ok := allIndexes[index.Name]; ok && coll == collectionName { continue } @@ -786,20 +796,13 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c pgIndexName = fmt.Sprintf("%s%s", tableNamePart, indexNamePart) // indexes must be unique across the whole database, so we check for duplicates for all collections - var duplicate bool - - for _, coll := range db { - if slices.ContainsFunc(coll.Indexes, func(i IndexInfo) bool { return pgIndexName == i.PgIndex }) { - duplicate = true - s++ - - break - } - } + _, duplicate := allPgIndexes[pgIndexName] if !duplicate { break } + + s++ } index.PgIndex = pgIndexName diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index d927cc885050..92ded30f76b8 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -466,97 +466,107 @@ func TestIndexesCreateDrop(t *testing.T) { collection, err := r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - t.Run("NonUniqueIndex", func(t *testing.T) { - i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { - return ii.Name == "index_non_unique" - }) - require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Indexes[i].PgIndex - - var sql string - err = db.QueryRow( - ctx, - "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, tableIndexName, - ).Scan(&sql) - require.NoError(t, err) - - expected := fmt.Sprintf( - `CREATE INDEX %s ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, - tableIndexName, dbName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) + t.Run("Lala", func(t *testing.T) { + t.Run("NonUniqueIndex", func(t *testing.T) { + t.Parallel() + + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { + return ii.Name == "index_non_unique" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Indexes[i].PgIndex + + var sql string + err := db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, tableIndexName, + ).Scan(&sql) + require.NoError(t, err) - t.Run("UniqueIndex", func(t *testing.T) { - i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { - return ii.Name == "index_unique" + expected := fmt.Sprintf( + `CREATE INDEX %s ON %q.%s USING btree (((_jsonb -> 'f1'::text)), ((_jsonb -> 'f2'::text)) DESC)`, + tableIndexName, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) }) - require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Indexes[i].PgIndex - var sql string - err = db.QueryRow( - ctx, - "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, tableIndexName, - ).Scan(&sql) - require.NoError(t, err) - - expected := fmt.Sprintf( - `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, - tableIndexName, dbName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) + t.Run("UniqueIndex", func(t *testing.T) { + t.Parallel() + + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { + return ii.Name == "index_unique" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Indexes[i].PgIndex + + var sql string + err := db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, tableIndexName, + ).Scan(&sql) + require.NoError(t, err) - t.Run("NestedFields", func(t *testing.T) { - i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { - return ii.Name == "nested_fields" + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> 'foo'::text)))`, + tableIndexName, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) }) - require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Indexes[i].PgIndex - var sql string - err = db.QueryRow( - ctx, - "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, tableIndexName, - ).Scan(&sql) - require.NoError(t, err) - - expected := fmt.Sprintf( - `CREATE INDEX %s ON %q.%s USING btree`+ - ` ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, - tableIndexName, dbName, collection.TableName, - ) - require.Equal(t, expected, sql) - }) + t.Run("NestedFields", func(t *testing.T) { + t.Parallel() + + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { + return ii.Name == "nested_fields" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Indexes[i].PgIndex + + var sql string + err := db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, tableIndexName, + ).Scan(&sql) + require.NoError(t, err) - t.Run("DefaultIndex", func(t *testing.T) { - i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { - return ii.Name == "_id_" + expected := fmt.Sprintf( + `CREATE INDEX %s ON %q.%s USING btree`+ + ` ((((_jsonb -> 'foo'::text) -> 'bar'::text)), (((_jsonb -> 'foo'::text) -> 'baz'::text)) DESC)`, + tableIndexName, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) }) - require.GreaterOrEqual(t, i, 0) - tableIndexName := collection.Indexes[i].PgIndex - var sql string - err = db.QueryRow( - ctx, - "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", - dbName, collection.TableName, tableIndexName, - ).Scan(&sql) - require.NoError(t, err) + t.Run("DefaultIndex", func(t *testing.T) { + t.Parallel() + + i := slices.IndexFunc(collection.Indexes, func(ii IndexInfo) bool { + return ii.Name == "_id_" + }) + require.GreaterOrEqual(t, i, 0) + tableIndexName := collection.Indexes[i].PgIndex + + var sql string + err := db.QueryRow( + ctx, + "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", + dbName, collection.TableName, tableIndexName, + ).Scan(&sql) + require.NoError(t, err) - expected := fmt.Sprintf( - `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, - tableIndexName, dbName, collection.TableName, - ) - require.Equal(t, expected, sql) + expected := fmt.Sprintf( + `CREATE UNIQUE INDEX %s ON %q.%s USING btree (((_jsonb -> '_id'::text)))`, + tableIndexName, dbName, collection.TableName, + ) + require.Equal(t, expected, sql) + }) }) t.Run("CheckSettingsAfterCreation", func(t *testing.T) { - err = r.initCollections(ctx, dbName, db) + err := r.initCollections(ctx, dbName, db) require.NoError(t, err) var refreshedCollection *Collection @@ -583,7 +593,7 @@ func TestIndexesCreateDrop(t *testing.T) { t.Run("DropIndexes", func(t *testing.T) { toDrop := []string{"index_non_unique", "nested_fields"} - err = r.IndexesDrop(ctx, dbName, collectionName, toDrop) + err := r.IndexesDrop(ctx, dbName, collectionName, toDrop) require.NoError(t, err) q := "SELECT count(indexdef) FROM pg_indexes WHERE schemaname = $1 AND tablename = $2" @@ -617,7 +627,7 @@ func TestIndexesCreateDrop(t *testing.T) { t.Parallel() var sql string - err = db.QueryRow( + err := db.QueryRow( ctx, "SELECT indexdef FROM pg_indexes WHERE schemaname = $1 AND tablename = $2 AND indexname = $3", dbName, metadataTableName, metadataTableName+"_id_idx", From b03d9275f6147d54ea3943f6252b2d182773a94f Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 14:09:16 +0200 Subject: [PATCH 35/39] wip --- internal/backends/postgresql/metadata/registry.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 24eed2bf3d59..229f19488c8b 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -755,8 +755,8 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c panic("collection does not exist") } - allIndexes := make(map[string]string, len(db)) - allPgIndexes := make(map[string]string, len(db)) + allIndexes := make(map[string]string, len(db)) // to check if the index already exists + allPgIndexes := make(map[string]string, len(db)) // to ensure there are no indexes with the same name in the pg schema for _, coll := range db { for _, index := range coll.Indexes { From 4ebd31104e0a78a8246e316e0cb65776e7e06e19 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 15:32:18 +0200 Subject: [PATCH 36/39] wip --- .../postgresql/metadata/registry_test.go | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 92ded30f76b8..67f294b45277 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -148,21 +148,6 @@ func TestCheckAuth(t *testing.T) { } } -func TestCreateDrop(t *testing.T) { - if testing.Short() { - t.Skip("skipping in -short mode") - } - - t.Parallel() - - connInfo := conninfo.New() - ctx := conninfo.Ctx(testutil.Ctx(t), connInfo) - - r, db, dbName := createDatabase(t, ctx) - collectionName := testutil.CollectionName(t) - testCollection(t, ctx, r, db, dbName, collectionName) -} - func TestCreateDropStress(t *testing.T) { if testing.Short() { t.Skip("skipping in -short mode") @@ -466,7 +451,7 @@ func TestIndexesCreateDrop(t *testing.T) { collection, err := r.CollectionGet(ctx, dbName, collectionName) require.NoError(t, err) - t.Run("Lala", func(t *testing.T) { + t.Run("CreateIndexes", func(t *testing.T) { t.Run("NonUniqueIndex", func(t *testing.T) { t.Parallel() @@ -566,6 +551,8 @@ func TestIndexesCreateDrop(t *testing.T) { }) t.Run("CheckSettingsAfterCreation", func(t *testing.T) { + t.Parallel() + err := r.initCollections(ctx, dbName, db) require.NoError(t, err) From ee8a461c4ffe2e8d067329176acb0d49221a0bae Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 15:44:36 +0200 Subject: [PATCH 37/39] wip --- internal/backends/postgresql/metadata/registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go index 229f19488c8b..96b95c098971 100644 --- a/internal/backends/postgresql/metadata/registry.go +++ b/internal/backends/postgresql/metadata/registry.go @@ -793,7 +793,7 @@ func (r *Registry) indexesCreate(ctx context.Context, p *pgxpool.Pool, dbName, c indexNamePart = indexNamePart[:l] } - pgIndexName = fmt.Sprintf("%s%s", tableNamePart, indexNamePart) + pgIndexName = fmt.Sprintf("%s_%s", tableNamePart, indexNamePart) // indexes must be unique across the whole database, so we check for duplicates for all collections _, duplicate := allPgIndexes[pgIndexName] From bc356809e1a664ab6bbf20fc4819381ca3e6bc42 Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 17:47:07 +0200 Subject: [PATCH 38/39] wip --- internal/backends/postgresql/metadata/registry_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/backends/postgresql/metadata/registry_test.go b/internal/backends/postgresql/metadata/registry_test.go index 67f294b45277..6ca89e690416 100644 --- a/internal/backends/postgresql/metadata/registry_test.go +++ b/internal/backends/postgresql/metadata/registry_test.go @@ -551,8 +551,6 @@ func TestIndexesCreateDrop(t *testing.T) { }) t.Run("CheckSettingsAfterCreation", func(t *testing.T) { - t.Parallel() - err := r.initCollections(ctx, dbName, db) require.NoError(t, err) From 97e89f74e6bff4880592eeebd97098f356ba051a Mon Sep 17 00:00:00 2001 From: Elena Grahovac Date: Wed, 4 Oct 2023 17:51:58 +0200 Subject: [PATCH 39/39] wip --- internal/backends/postgresql/database_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backends/postgresql/database_test.go b/internal/backends/postgresql/database_test.go index 3df4aa5ed4f9..e54e19cc1ffc 100644 --- a/internal/backends/postgresql/database_test.go +++ b/internal/backends/postgresql/database_test.go @@ -72,7 +72,7 @@ func TestDatabaseStats(t *testing.T) { require.NotZero(t, res.SizeCollections) require.Zero(t, res.CountObjects) require.Zero(t, res.CountIndexes) - require.Zero(t, res.SizeIndexes) + require.NotZero(t, res.SizeIndexes) // includes metadata table's indexes }) t.Run("DatabaseWithCollections", func(t *testing.T) {