Skip to content

Commit

Permalink
fix incorrect CREATE GSI syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
@miyamo2
miyamo2 committed Jun 22, 2024
1 parent 9369e26 commit 6869d82
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 62 deletions.
115 changes: 60 additions & 55 deletions migrator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,68 +234,73 @@ func (m Migrator) HasConstraint(dst interface{}, name string) bool {
}

func (m Migrator) CreateIndex(dst interface{}, name string) error {
var (
wcu, rcu int
)

if ws, ok := dst.(CapacityUnitsSpecifier); ok {
wcu = ws.WCU()
rcu = ws.RCU()
}

rv := reflect.ValueOf(dst)
var rt reflect.Type
switch rv.Kind() {
case reflect.Pointer:
rt = reflect.TypeOf(reflect.ValueOf(dst).Elem().Interface())
default:
rt = reflect.TypeOf(dst)
}

td := newDynmgrmTableDefine(rt)
if name != "" {
v, ok := td.GSI[name]
if !ok {
return fmt.Errorf("gsi '%s' is not defined in %T", name, dst)
return m.base.RunWithValue(dst, func(stmt *gorm.Statement) (err error) {
var (
wcu, rcu int
)

if ws, ok := dst.(CapacityUnitsSpecifier); ok {
wcu = ws.WCU()
rcu = ws.RCU()
}
td.GSI = map[string]*dynmgrmSecondaryIndexDefine{name: v}
}
for k, v := range td.GSI {
// `CREATE GSI` are proprietary PartiQL syntax by btnguyen2k/godynamo
// This is why place holder/bind variables are not used.
ddlBuilder := strings.Builder{}
ddlBuilder.WriteString(fmt.Sprintf(`CREATE GSI IF NOT EXISTS %s `, k))
ddlBuilder.WriteString(fmt.Sprintf(`WITH PK=%s:%s`, v.PK.Name, v.PK.DataType))
if skn := v.SK.Name; skn != "" {
ddlBuilder.WriteString(fmt.Sprintf(`, WITH SK=%s:%s`, skn, v.SK.DataType))
}
opts := make([]string, 0, 3)
if wcu > 0 {
opts = append(opts, fmt.Sprintf(`WITH wcu=%d`, wcu))
}
if rcu > 0 {
opts = append(opts, fmt.Sprintf(`WITH rcu=%d`, rcu))

rv := reflect.ValueOf(dst)
var rt reflect.Type
switch rv.Kind() {
case reflect.Pointer:
rt = reflect.TypeOf(reflect.ValueOf(dst).Elem().Interface())
default:
rt = reflect.TypeOf(dst)

Check warning on line 253 in migrator.go

View check run for this annotation

Codecov / codecov/patch

migrator.go#L252-L253 

Added lines #L252 - L253 were not covered by tests
}

projective := slices.DeleteFunc(append(td.NonKeyAttr, td.PK.Name, td.SK.Name), func(s string) bool {
if s == v.PK.Name || s == v.SK.Name {
return true
td := newDynmgrmTableDefine(rt)
if name != "" {
v, ok := td.GSI[name]
if !ok {
return fmt.Errorf("gsi '%s' is not defined in %T", name, dst)
}
return slices.Contains(v.NonProjectiveAttrs, s)
})
if len(projective) > 0 {
opts = append(opts, fmt.Sprintf(`WITH projection=%s`, strings.Join(projective, ",")))
td.GSI = map[string]*dynmgrmSecondaryIndexDefine{name: v}
}
for k, v := range td.GSI {
// `CREATE GSI` are proprietary PartiQL syntax by btnguyen2k/godynamo
// This is why place holder/bind variables are not used.
ddlBuilder := strings.Builder{}
ddlBuilder.WriteString(fmt.Sprintf(`CREATE GSI IF NOT EXISTS %s ON %s `, k, m.currentTable(stmt)))
ddlBuilder.WriteString(fmt.Sprintf(`WITH PK=%s:%s`, v.PK.Name, v.PK.DataType))
if skn := v.SK.Name; skn != "" {
ddlBuilder.WriteString(fmt.Sprintf(`, WITH SK=%s:%s`, skn, v.SK.DataType))
}
opts := make([]string, 0, 3)
if wcu > 0 {
opts = append(opts, fmt.Sprintf(`WITH wcu=%d`, wcu))
}
if rcu > 0 {
opts = append(opts, fmt.Sprintf(`WITH rcu=%d`, rcu))
}

for _, o := range opts {
ddlBuilder.WriteString(", ")
ddlBuilder.WriteString(o)
}
if err := m.db.Exec(ddlBuilder.String()).Error; err != nil {
return err
projective := make([]string, 0)
if len(v.NonProjectiveAttrs) != 0 {
projective = slices.DeleteFunc(append(td.NonKeyAttr, td.PK.Name, td.SK.Name), func(s string) bool {
if s == v.PK.Name || s == v.SK.Name {
return true
}
return slices.Contains(v.NonProjectiveAttrs, s)
})
}
if len(projective) > 0 {
opts = append(opts, fmt.Sprintf(`WITH projection=%s`, strings.Join(projective, ",")))
}

for _, o := range opts {
ddlBuilder.WriteString(", ")
ddlBuilder.WriteString(o)
}
if err := m.db.Exec(ddlBuilder.String()).Error; err != nil {
return err
}
}
}
return nil
return nil
})
}

func (m Migrator) DropIndex(dst interface{}, name string) error {
Expand Down
47 changes: 40 additions & 7 deletions migrator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,10 @@ func TestMigrator_CreateIndex(t *testing.T) {
name string
}
type test struct {
args args
mockDBExecOptions []func(*mockDBExecProp)
want error
args args
mockDBExecOptions []func(*mockDBExecProp)
mockBaseMigratorCurrentTableOptions []func(*mockBaseMigratorCurrentTableProp)
want error
}
errDBExec := errors.New("db exec error")
tests := map[string]test{
Expand All @@ -265,44 +266,60 @@ func TestMigrator_CreateIndex(t *testing.T) {
mockDBExecOptions: []func(*mockDBExecProp){
mockDBForMigratorExecWithArgs(t,
mockDBExecArgs{
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index ON create_table_a_tables WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
nil}),
mockDBForMigratorExecWithTimes(t, 1),
mockDBForMigratorExecWithResult(t, &gorm.DB{}),
},
mockBaseMigratorCurrentTableOptions: []func(*mockBaseMigratorCurrentTableProp){
mockBaseMigratorCurrentTableWithResult(t, clause.Table{Name: "create_table_a_tables"}),
mockBaseMigratorCurrentTableWithTimes(t, 1),
},
},
"happy_path/physical": {
args: args{&CreateTableATable{}, ""},
mockDBExecOptions: []func(*mockDBExecProp){
mockDBForMigratorExecWithArgs(t,
mockDBExecArgs{
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index ON create_table_a_tables WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
nil}),
mockDBForMigratorExecWithTimes(t, 1),
mockDBForMigratorExecWithResult(t, &gorm.DB{}),
},
mockBaseMigratorCurrentTableOptions: []func(*mockBaseMigratorCurrentTableProp){
mockBaseMigratorCurrentTableWithResult(t, clause.Table{Name: "create_table_a_tables"}),
mockBaseMigratorCurrentTableWithTimes(t, 1),
},
},
"happy_path/with_name": {
args: args{&CreateTableATable{}, "gsi_pk-gsi_sk-index"},
mockDBExecOptions: []func(*mockDBExecProp){
mockDBForMigratorExecWithArgs(t,
mockDBExecArgs{
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index ON create_table_a_tables WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
nil}),
mockDBForMigratorExecWithTimes(t, 1),
mockDBForMigratorExecWithResult(t, &gorm.DB{}),
},
mockBaseMigratorCurrentTableOptions: []func(*mockBaseMigratorCurrentTableProp){
mockBaseMigratorCurrentTableWithResult(t, clause.Table{Name: "create_table_a_tables"}),
mockBaseMigratorCurrentTableWithTimes(t, 1),
},
},
"unhappy_path/db_exec_returns_error": {
args: args{&CreateTableATable{}, ""},
mockDBExecOptions: []func(*mockDBExecProp){
mockDBForMigratorExecWithArgs(t,
mockDBExecArgs{
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
`CREATE GSI IF NOT EXISTS gsi_pk-gsi_sk-index ON create_table_a_tables WITH PK=gsi_pk:string, WITH SK=gsi_sk:string, WITH wcu=10, WITH rcu=10, WITH projection=lsi_sk,projective_attr_1,projective_attr_2,pk,sk`,
nil}),
mockDBForMigratorExecWithTimes(t, 1),
mockDBForMigratorExecWithResult(t, &gorm.DB{Error: errDBExec}),
},
mockBaseMigratorCurrentTableOptions: []func(*mockBaseMigratorCurrentTableProp){
mockBaseMigratorCurrentTableWithResult(t, clause.Table{Name: "create_table_a_tables"}),
mockBaseMigratorCurrentTableWithTimes(t, 1),
},
want: errDBExec,
},
"unhappy_path/with-undefined-gsi-name": {
Expand All @@ -316,11 +333,27 @@ func TestMigrator_CreateIndex(t *testing.T) {
defer ctrl.Finish()
mdb := mocks.NewMockDBForMigrator(ctrl)
mbm := mocks.NewMockBaseMigrator(ctrl)
mbm.EXPECT().RunWithValue(gomock.Any(), gomock.Any()).DoAndReturn(
func(model interface{}, f func(stmt *gorm.Statement) error) error {
return f(nil)
},
).Times(1)

ctp := mockBaseMigratorCurrentTableProp{}
for _, o := range tt.mockBaseMigratorCurrentTableOptions {
o(&ctp)
}
mbmxp := mbm.EXPECT().CurrentTable(gomock.Any())
if cmp.Diff(ctp.result, clause.Table{}) != "" {
mbmxp = mbmxp.Return(ctp.result)
}
mbmxp.Times(ctp.times)

ep := mockDBExecProp{}
for _, o := range tt.mockDBExecOptions {
o(&ep)
}

var dbExecCall *gomock.Call
if ep.args != nil {
dbExecCall = mdb.EXPECT().Exec(ep.args.sql, ep.args.values...)
Expand Down

0 comments on commit 6869d82

Please sign in to comment.