[BUG] Changes to schema for sysdb. (#2897)

[BUG] Schema changes related to Issue#2884 and
few other necessary constraints.

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
  - Fix for Issue #2884.
  - Added additional constraints to the schema to avoid future issues.
 - New functionality
	 - n/a

## Test plan
*How are these changes tested?*

- [x] Tests pass locally with `make test` for golang

## Documentation Changes
None

go/go.sum

@@ -73,6 +73,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
 github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
 github.com/Microsoft/hcsshim v0.11.4 h1:68vKo2VN8DE9AdN4tnkWnmdhqdbpUFM8OF3Airm7fz8=
 github.com/Microsoft/hcsshim v0.11.4/go.mod h1:smjE4dvqPX9Zldna+t5FG3rnoHhaB7QYxPRqGcpAD9w=
+github.com/alecthomas/kong v0.7.1 h1:azoTh0IOfwlAX3qN9sHWTxACE2oV8Bg2gAwBsMwDQY4=
+github.com/alecthomas/kong v0.7.1/go.mod h1:n1iCIO2xS46oE8ZfYCNDqdR0b0wZNrXAIAqro/2132U=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=

go/migrations/20241003212820.sql

@@ -0,0 +1,6 @@
+-- Modify "collections" table
+ALTER TABLE "public"."collections" ALTER COLUMN "name" SET NOT NULL, ALTER COLUMN "database_id" SET NOT NULL;
+-- Modify "databases" table
+ALTER TABLE "public"."databases" ALTER COLUMN "name" SET NOT NULL, ALTER COLUMN "tenant_id" SET NOT NULL;
+-- Modify "segments" table
+ALTER TABLE "public"."segments" ADD CONSTRAINT "uni_segments_id" UNIQUE ("id");

go/migrations/atlas.sum

@@ -1,8 +1,9 @@
-h1:4X2Y8W9SS2BVW59Ku/FrBk9PLzrQe/Zx0vD1oha/5to=
+h1:Xacc5EyAVCwyNNuFYFQa/HxjtROca0tOu21L3UKvm70=
 20240313233558.sql h1:Gv0TiSYsqGoOZ2T2IWvX4BOasauxool8PrBOIjmmIdg=
 20240321194713.sql h1:kVkNpqSFhrXGVGFFvL7JdK3Bw31twFcEhI6A0oCFCkg=
 20240327075032.sql h1:nlr2J74XRU8erzHnKJgMr/tKqJxw9+R6RiiEBuvuzgo=
 20240327172649.sql h1:UUGo6AzWXKLcpYVd5qH6Hv9jpHNV86z42o6ft5OR0zU=
 20240411201006.sql h1:jjzYJPzDVTxQAvOI7gRtNTiZJHy1Hpw5urP8EzqxgUk=
 20240612201006.sql h1:vUuh/O0blyoOYS+YEjo6/zqRmwtoaleNEUqKCAecxKU=
 20240621171854.sql h1:kc8ZFK8A4/GLHu0gQ04RdIt3O38wfcD6ouXX4nr7lTk=
+20241003212820.sql h1:zHloxrMr7EMcqV008a3aqQdU5fHjWY3m66CIoThexbo=

go/pkg/common/errors.go

@@ -12,6 +12,7 @@ var (
 	// Database errors
 	ErrDatabaseNotFound                  = errors.New("database not found")
 	ErrDatabaseUniqueConstraintViolation = errors.New("database unique constraint violation")
+	ErrDatabaseNameEmpty                 = errors.New("database name is empty")
 
 	// Collection errors
 	ErrCollectionNotFound                    = errors.New("collection not found")

go/pkg/coordinator/apis_test.go

@@ -273,6 +273,32 @@ func (suite *APIsTestSuite) TestCreateGetDeleteCollections() {
 	})
 	suite.Error(err)
 
+	// Create collection with empty name fails
+	_, _, err = suite.coordinator.CreateCollection(ctx, &model.CreateCollection{
+		ID:           suite.sampleCollections[0].ID,
+		Name:         "",
+		TenantID:     suite.tenantName,
+		DatabaseName: suite.databaseName,
+	})
+	suite.Error(err)
+
+	// Create collection with empty tenant id fails
+	_, _, err = suite.coordinator.CreateCollection(ctx, &model.CreateCollection{
+		ID:           suite.sampleCollections[0].ID,
+		Name:         suite.sampleCollections[0].Name,
+		DatabaseName: suite.databaseName,
+	})
+	suite.Error(err)
+
+	// Create collection with random tenant id fails
+	_, _, err = suite.coordinator.CreateCollection(ctx, &model.CreateCollection{
+		ID:           suite.sampleCollections[0].ID,
+		Name:         suite.sampleCollections[0].Name,
+		TenantID:     "random_tenant_id",
+		DatabaseName: suite.databaseName,
+	})
+	suite.Error(err)
+
 	// Find by name
 	for _, collection := range suite.sampleCollections {
 		result, err := suite.coordinator.GetCollections(ctx, types.NilUniqueID(), &collection.Name, suite.tenantName, suite.databaseName, nil, nil)
@@ -384,7 +410,32 @@ func (suite *APIsTestSuite) TestCreateUpdateWithDatabase() {
 	ctx := context.Background()
 	newDatabaseName := "test_apis_CreateUpdateWithDatabase"
 	newDatabaseId := uuid.New().String()
+	// Create database with empty string in name fails
 	_, err := suite.coordinator.CreateDatabase(ctx, &model.CreateDatabase{
+		ID:     newDatabaseId,
+		Name:   "",
+		Tenant: suite.tenantName,
+	})
+	suite.Error(err)
+
+	// Create database with empty string in tenant fails
+	_, err = suite.coordinator.CreateDatabase(ctx, &model.CreateDatabase{
+		ID:     newDatabaseId,
+		Name:   newDatabaseName,
+		Tenant: "",
+	})
+	suite.Error(err)
+
+	// Create database with random non-existent tenant id fails
+	_, err = suite.coordinator.CreateDatabase(ctx, &model.CreateDatabase{
+		ID:     newDatabaseId,
+		Name:   newDatabaseName,
+		Tenant: "random_tenant_id",
+	})
+	suite.Error(err)
+
+	// Correct creation
+	_, err = suite.coordinator.CreateDatabase(ctx, &model.CreateDatabase{
 		ID:     newDatabaseId,
 		Name:   newDatabaseName,
 		Tenant: suite.tenantName,
@@ -691,6 +742,27 @@ func (suite *APIsTestSuite) TestCreateGetDeleteSegments() {
 			Metadata:     segment.Metadata,
 		})
 		suite.NoError(errSegmentCreation)
+
+		// Create segment with empty collection id fails
+		errSegmentCreation = c.CreateSegment(ctx, &model.CreateSegment{
+			ID:           segment.ID,
+			Type:         segment.Type,
+			Scope:        segment.Scope,
+			CollectionID: types.NilUniqueID(),
+			Metadata:     segment.Metadata,
+		})
+		suite.Error(errSegmentCreation)
+
+		// Create segment to test unique constraint violation on segment.id.
+		// This should fail because the id is already taken.
+		errSegmentCreation = c.CreateSegment(ctx, &model.CreateSegment{
+			ID:           segment.ID,
+			Type:         segment.Type,
+			Scope:        segment.Scope,
+			CollectionID: types.MustParse("00000000-d7d7-413b-92e1-731098a6e777"),
+			Metadata:     segment.Metadata,
+		})
+		suite.Error(errSegmentCreation)
 	}
 
 	var results []*model.Segment

go/pkg/metastore/coordinator/table_catalog.go

@@ -98,7 +98,23 @@ func (tc *Catalog) ResetState(ctx context.Context) error {
 func (tc *Catalog) CreateDatabase(ctx context.Context, createDatabase *model.CreateDatabase, ts types.Timestamp) (*model.Database, error) {
 	var result *model.Database
 
-	err := tc.txImpl.Transaction(ctx, func(txCtx context.Context) error {
+	// Check if database name is not empty
+	if createDatabase.Name == "" {
+		return nil, common.ErrDatabaseNameEmpty
+	}
+
+	// Check if tenant exists for the given tenant id
+	tenants, err := tc.metaDomain.TenantDb(ctx).GetTenants(createDatabase.Tenant)
+	if err != nil {
+		log.Error("error getting tenants", zap.Error(err))
+		return nil, err
+	}
+	if len(tenants) == 0 {
+		log.Error("tenant not found", zap.Error(err))
+		return nil, common.ErrTenantNotFound
+	}
+
+	err = tc.txImpl.Transaction(ctx, func(txCtx context.Context) error {
 		dbDatabase := &dbmodel.Database{
 			ID:       createDatabase.ID,
 			Name:     createDatabase.Name,

go/pkg/metastore/db/dao/database.go

@@ -2,6 +2,7 @@ package dao
 
 import (
 	"errors"
+
 	"github.com/chroma-core/chroma/go/pkg/common"
 	"github.com/chroma-core/chroma/go/pkg/metastore/db/dbmodel"
 	"github.com/jackc/pgx/v5/pgconn"
@@ -54,7 +55,7 @@ func (s *databaseDb) GetDatabases(tenantID string, databaseName string) ([]*dbmo
 func (s *databaseDb) Insert(database *dbmodel.Database) error {
 	err := s.db.Create(database).Error
 	if err != nil {
-		log.Error("create tenant failed", zap.Error(err))
+		log.Error("insert database failed", zap.Error(err))
 		var pgErr *pgconn.PgError
 		ok := errors.As(err, &pgErr)
 		if ok {

go/pkg/metastore/db/dbmodel/collection.go

@@ -8,10 +8,10 @@ import (
 
 type Collection struct {
 	ID                   string          `gorm:"id;primaryKey"`
-	Name                 *string         `gorm:"name;index:idx_name,unique;"`
+	Name                 *string         `gorm:"name;not null;index:idx_name,unique;"`
 	ConfigurationJsonStr *string         `gorm:"configuration_json_str"`
 	Dimension            *int32          `gorm:"dimension"`
-	DatabaseID           string          `gorm:"database_id;index:idx_name,unique;"`
+	DatabaseID           string          `gorm:"database_id;not null;index:idx_name,unique;"`
 	Ts                   types.Timestamp `gorm:"ts;type:bigint;default:0"`
 	IsDeleted            bool            `gorm:"is_deleted;type:bool;default:false"`
 	CreatedAt            time.Time       `gorm:"created_at;type:timestamp;not null;default:current_timestamp"`

go/pkg/metastore/db/dbmodel/database.go

@@ -8,8 +8,8 @@ import (
 
 type Database struct {
 	ID        string          `gorm:"id;primaryKey;unique"`
-	Name      string          `gorm:"name;type:varchar(128);not_null;uniqueIndex:idx_tenantid_name"`
-	TenantID  string          `gorm:"tenant_id;type:varchar(128);not_null;uniqueIndex:idx_tenantid_name"`
+	Name      string          `gorm:"name;type:varchar(128);not null;uniqueIndex:idx_tenantid_name"`
+	TenantID  string          `gorm:"tenant_id;type:varchar(128);not null;uniqueIndex:idx_tenantid_name"`
 	Ts        types.Timestamp `gorm:"ts;type:bigint;default:0"`
 	IsDeleted bool            `gorm:"is_deleted;type:bool;default:false"`
 	CreatedAt time.Time       `gorm:"created_at;type:timestamp;not null;default:current_timestamp"`

go/pkg/metastore/db/dbmodel/segment.go

@@ -14,9 +14,9 @@ type Segment struct {
 	   need to modify CollectionID in the near future. Each Segment should always have a
 	   collection as a parent and cannot be modified. */
 	CollectionID *string             `gorm:"collection_id;primaryKey;not null"`
-	ID           string              `gorm:"id;primaryKey"`
+	ID           string              `gorm:"id;primaryKey;unique;not null"`
 	Type         string              `gorm:"type;type:string;not null"`
-	Scope        string              `gorm:"scope"`
+	Scope        string              `gorm:"scope;"`
 	Ts           types.Timestamp     `gorm:"ts;type:bigint;default:0"`
 	IsDeleted    bool                `gorm:"is_deleted;type:bool;default:false"`
 	CreatedAt    time.Time           `gorm:"created_at;type:timestamp;not null;default:current_timestamp"`