Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: Split existing alter operations #2156

Merged
merged 4 commits into from
Oct 30, 2023
Merged

chore: Split existing alter operations #2156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf848cd
Extract set and unset tags
sfc-gh-asawicki Oct 25, 2023
7fee820
Fix warehouse validation
sfc-gh-asawicki Oct 30, 2023
bf0918d
Fix after review
sfc-gh-asawicki Oct 30, 2023
592c7f8
Fix warehouses
sfc-gh-asawicki Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions pkg/resources/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,7 @@ func UpdateSchema(d *schema.ResourceData, meta interface{}) error {
unsetTags[i] = sdk.NewDatabaseObjectIdentifier(t.database, t.name)
}
err := client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Unset: &sdk.SchemaUnset{
Tag: unsetTags,
},
UnsetTag: unsetTags,
})
if err != nil {
return fmt.Errorf("error dropping tags on %v", d.Id())
Expand All @@ -259,9 +257,7 @@ func UpdateSchema(d *schema.ResourceData, meta interface{}) error {
}
}
err = client.Schemas.Alter(ctx, id, &sdk.AlterSchemaOptions{
Set: &sdk.SchemaSet{
Tag: setTags,
},
SetTag: setTags,
})
if err != nil {
return fmt.Errorf("error setting tags on %v", d.Id())
Expand Down
24 changes: 12 additions & 12 deletions pkg/sdk/accounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,21 @@ type AlterAccountOptions struct {
alter bool `ddl:"static" sql:"ALTER"`
account bool `ddl:"static" sql:"ACCOUNT"`

Set *AccountSet `ddl:"keyword" sql:"SET"`
Unset *AccountUnset `ddl:"list,no_parentheses" sql:"UNSET"`
Rename *AccountRename `ddl:"-"`
Drop *AccountDrop `ddl:"-"`
Set *AccountSet `ddl:"keyword" sql:"SET"`
Unset *AccountUnset `ddl:"list,no_parentheses" sql:"UNSET"`
SetTag []TagAssociation `ddl:"keyword" sql:"SET TAG"`
UnsetTag []ObjectIdentifier `ddl:"keyword" sql:"UNSET TAG"`
Rename *AccountRename `ddl:"-"`
Drop *AccountDrop `ddl:"-"`
}

func (opts *AlterAccountOptions) validate() error {
if opts == nil {
return errors.Join(ErrNilOptions)
}
var errs []error
if !exactlyOneValueSet(opts.Set, opts.Unset, opts.Drop, opts.Rename) {
errs = append(errs, errExactlyOneOf("CreateAccountOptions", "Set", "Unset", "Drop", "Rename"))
if !exactlyOneValueSet(opts.Set, opts.Unset, opts.SetTag, opts.UnsetTag, opts.Drop, opts.Rename) {
errs = append(errs, errExactlyOneOf("CreateAccountOptions", "Set", "Unset", "SetTag", "UnsetTag", "Drop", "Rename"))
}
if valueSet(opts.Set) {
if err := opts.Set.validate(); err != nil {
Expand Down Expand Up @@ -164,13 +166,12 @@ type AccountSet struct {
ResourceMonitor AccountObjectIdentifier `ddl:"identifier,equals" sql:"RESOURCE_MONITOR"`
PasswordPolicy SchemaObjectIdentifier `ddl:"identifier" sql:"PASSWORD POLICY"`
SessionPolicy SchemaObjectIdentifier `ddl:"identifier" sql:"SESSION POLICY"`
Tag []TagAssociation `ddl:"keyword" sql:"TAG"`
}

func (opts *AccountSet) validate() error {
var errs []error
if !exactlyOneValueSet(opts.Parameters, opts.ResourceMonitor, opts.PasswordPolicy, opts.SessionPolicy, opts.Tag) {
errs = append(errs, errExactlyOneOf("AccountSet", "Parameters", "ResourceMonitor", "PasswordPolicy", "SessionPolicy", "Tag"))
if !exactlyOneValueSet(opts.Parameters, opts.ResourceMonitor, opts.PasswordPolicy, opts.SessionPolicy) {
errs = append(errs, errExactlyOneOf("AccountSet", "Parameters", "ResourceMonitor", "PasswordPolicy", "SessionPolicy"))
}
if valueSet(opts.Parameters) {
if err := opts.Parameters.validate(); err != nil {
Expand Down Expand Up @@ -198,13 +199,12 @@ type AccountUnset struct {
Parameters *AccountLevelParametersUnset `ddl:"list,no_parentheses"`
PasswordPolicy *bool `ddl:"keyword" sql:"PASSWORD POLICY"`
SessionPolicy *bool `ddl:"keyword" sql:"SESSION POLICY"`
Tag []ObjectIdentifier `ddl:"keyword" sql:"TAG"`
}

func (opts *AccountUnset) validate() error {
var errs []error
if !exactlyOneValueSet(opts.Parameters, opts.PasswordPolicy, opts.SessionPolicy, opts.Tag) {
errs = append(errs, errExactlyOneOf("AccountUnset", "Parameters", "PasswordPolicy", "SessionPolicy", "Tag"))
if !exactlyOneValueSet(opts.Parameters, opts.PasswordPolicy, opts.SessionPolicy) {
errs = append(errs, errExactlyOneOf("AccountUnset", "Parameters", "PasswordPolicy", "SessionPolicy"))
}
if valueSet(opts.Parameters) {
if err := opts.Parameters.validate(); err != nil {
Expand Down
24 changes: 10 additions & 14 deletions pkg/sdk/accounts_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,14 @@ func TestAccountAlter(t *testing.T) {

t.Run("with set tag", func(t *testing.T) {
opts := &AlterAccountOptions{
Set: &AccountSet{
Tag: []TagAssociation{
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag1"),
Value: "v1",
},
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag2"),
Value: "v2",
},
SetTag: []TagAssociation{
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag1"),
Value: "v1",
},
{
Name: NewSchemaObjectIdentifier("db", "schema", "tag2"),
Value: "v2",
},
},
}
Expand All @@ -158,10 +156,8 @@ func TestAccountAlter(t *testing.T) {

t.Run("with unset tag", func(t *testing.T) {
opts := &AlterAccountOptions{
Unset: &AccountUnset{
Tag: []ObjectIdentifier{
NewSchemaObjectIdentifier("db", "schema", "tag1"),
},
UnsetTag: []ObjectIdentifier{
NewSchemaObjectIdentifier("db", "schema", "tag1"),
},
}
assertOptsValidAndSQLEquals(t, opts, `ALTER ACCOUNT UNSET TAG "db"."schema"."tag1"`)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdk/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func errNotSet(structName string, fieldNames ...string) error {
}

func errExactlyOneOf(structName string, fieldNames ...string) error {
return newError(fmt.Sprintf("exactly one of %s fileds %v must be set", structName, fieldNames), 2)
return newError(fmt.Sprintf("exactly one of %s fields %v must be set", structName, fieldNames), 2)
}

func errAtLeastOneOf(structName string, fieldNames ...string) error {
Expand Down
22 changes: 11 additions & 11 deletions pkg/sdk/masking_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ type AlterMaskingPolicyOptions struct {
NewName *SchemaObjectIdentifier `ddl:"identifier" sql:"RENAME TO"`
Set *MaskingPolicySet `ddl:"keyword" sql:"SET"`
Unset *MaskingPolicyUnset `ddl:"keyword" sql:"UNSET"`
SetTag []TagAssociation `ddl:"keyword" sql:"SET TAG"`
UnsetTag []ObjectIdentifier `ddl:"keyword" sql:"UNSET TAG"`
}

func (opts *AlterMaskingPolicyOptions) validate() error {
Expand All @@ -112,8 +114,8 @@ func (opts *AlterMaskingPolicyOptions) validate() error {
if opts.NewName != nil && !ValidObjectIdentifier(opts.NewName) {
errs = append(errs, errInvalidIdentifier("AlterMaskingPolicyOptions", "NewName"))
}
if !exactlyOneValueSet(opts.NewName, opts.Set, opts.Unset) {
errs = append(errs, errExactlyOneOf("AlterMaskingPolicyOptions", "NewName", "Set", "Unset"))
if !exactlyOneValueSet(opts.Set, opts.Unset, opts.SetTag, opts.UnsetTag, opts.NewName) {
errs = append(errs, errExactlyOneOf("AlterMaskingPolicyOptions", "Set", "Unset", "SetTag", "UnsetTag", "NewName"))
}
if valueSet(opts.Set) {
if err := opts.Set.validate(); err != nil {
Expand All @@ -129,26 +131,24 @@ func (opts *AlterMaskingPolicyOptions) validate() error {
}

type MaskingPolicySet struct {
Body *string `ddl:"parameter,no_equals" sql:"BODY ->"`
Tag []TagAssociation `ddl:"keyword" sql:"TAG"`
Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"`
Body *string `ddl:"parameter,no_equals" sql:"BODY ->"`
Comment *string `ddl:"parameter,single_quotes" sql:"COMMENT"`
}

func (v *MaskingPolicySet) validate() error {
if !exactlyOneValueSet(v.Body, v.Tag, v.Comment) {
return errExactlyOneOf("MaskingPolicySet", "Body", "Tag", "Comment")
if !exactlyOneValueSet(v.Body, v.Comment) {
return errExactlyOneOf("MaskingPolicySet", "Body", "Comment")
}
return nil
}

type MaskingPolicyUnset struct {
Tag []ObjectIdentifier `ddl:"keyword" sql:"TAG"`
Comment *bool `ddl:"keyword" sql:"COMMENT"`
Comment *bool `ddl:"keyword" sql:"COMMENT"`
}

func (v *MaskingPolicyUnset) validate() error {
if !exactlyOneValueSet(v.Tag, v.Comment) {
return errExactlyOneOf("MaskingPolicyUnset", "Tag", "Comment")
if !exactlyOneValueSet(v.Comment) {
return errExactlyOneOf("MaskingPolicyUnset", "Comment")
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdk/masking_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestMaskingPolicyAlter(t *testing.T) {
opts := &AlterMaskingPolicyOptions{
name: id,
}
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterMaskingPolicyOptions", "NewName", "Set", "Unset"))
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterMaskingPolicyOptions", "Set", "Unset", "SetTag", "UnsetTag", "NewName"))
})

t.Run("with set", func(t *testing.T) {
Expand Down
18 changes: 5 additions & 13 deletions pkg/sdk/pipes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ type AlterPipeOptions struct {
name SchemaObjectIdentifier `ddl:"identifier"`

// One of
Set *PipeSet `ddl:"list,no_parentheses" sql:"SET"`
Unset *PipeUnset `ddl:"list,no_parentheses" sql:"UNSET"`
SetTags *PipeSetTags `ddl:"list,no_parentheses" sql:"SET TAG"`
UnsetTags *PipeUnsetTags `ddl:"list,no_parentheses" sql:"UNSET TAG"`
Refresh *PipeRefresh `ddl:"keyword" sql:"REFRESH"`
Set *PipeSet `ddl:"list,no_parentheses" sql:"SET"`
Unset *PipeUnset `ddl:"list,no_parentheses" sql:"UNSET"`
SetTag []TagAssociation `ddl:"keyword" sql:"SET TAG"`
UnsetTag []ObjectIdentifier `ddl:"keyword" sql:"UNSET TAG"`
Refresh *PipeRefresh `ddl:"keyword" sql:"REFRESH"`
}

type PipeSet struct {
Expand All @@ -60,14 +60,6 @@ type PipeUnset struct {
Comment *bool `ddl:"keyword" sql:"COMMENT"`
}

type PipeSetTags struct {
Tag []TagAssociation `ddl:"keyword"`
}

type PipeUnsetTags struct {
Tag []ObjectIdentifier `ddl:"keyword"`
}

type PipeRefresh struct {
Prefix *string `ddl:"parameter,single_quotes" sql:"PREFIX"`
ModifiedAfter *string `ddl:"parameter,single_quotes" sql:"MODIFIED_AFTER"`
Expand Down
62 changes: 19 additions & 43 deletions pkg/sdk/pipes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestPipesAlter(t *testing.T) {

t.Run("validation: no alter action", func(t *testing.T) {
opts := defaultOpts()
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTags", "UnsetTags", "Refresh"))
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTag", "UnsetTag", "Refresh"))
})

t.Run("validation: multiple alter actions", func(t *testing.T) {
Expand All @@ -81,7 +81,7 @@ func TestPipesAlter(t *testing.T) {
opts.Unset = &PipeUnset{
Comment: Bool(true),
}
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTags", "UnsetTags", "Refresh"))
assertOptsInvalidJoinedErrors(t, opts, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTag", "UnsetTag", "Refresh"))
})

t.Run("validation: no property to set", func(t *testing.T) {
Expand All @@ -90,53 +90,33 @@ func TestPipesAlter(t *testing.T) {
assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterPipeOptions.Set", "ErrorIntegration", "PipeExecutionPaused", "Comment"))
})

t.Run("validation: empty tags slice for set", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = &PipeSetTags{
Tag: []TagAssociation{},
}
assertOptsInvalidJoinedErrors(t, opts, errNotSet("AlterPipeOptions.SetTags", "Tag"))
})

t.Run("validation: no property to unset", func(t *testing.T) {
opts := defaultOpts()
opts.Unset = &PipeUnset{}
assertOptsInvalidJoinedErrors(t, opts, errAtLeastOneOf("AlterPipeOptions.Unset", "PipeExecutionPaused", "Comment"))
})

t.Run("validation: empty tags slice for unset", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = &PipeUnsetTags{
Tag: []ObjectIdentifier{},
}
assertOptsInvalidJoinedErrors(t, opts, errNotSet("AlterPipeOptions.UnsetTags", "Tag"))
})

t.Run("set tag: single", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = &PipeSetTags{
Tag: []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
opts.SetTag = []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s SET TAG "tag_name1" = 'v1'`, id.FullyQualifiedName())
})

t.Run("set tag: multiple", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = &PipeSetTags{
Tag: []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
{
Name: NewAccountObjectIdentifier("tag_name2"),
Value: "v2",
},
opts.SetTag = []TagAssociation{
{
Name: NewAccountObjectIdentifier("tag_name1"),
Value: "v1",
},
{
Name: NewAccountObjectIdentifier("tag_name2"),
Value: "v2",
},
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s SET TAG "tag_name1" = 'v1', "tag_name2" = 'v2'`, id.FullyQualifiedName())
Expand All @@ -155,21 +135,17 @@ func TestPipesAlter(t *testing.T) {

t.Run("unset tag: single", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = &PipeUnsetTags{
Tag: []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
},
opts.UnsetTag = []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s UNSET TAG "tag_name1"`, id.FullyQualifiedName())
})

t.Run("unset tag: multi", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = &PipeUnsetTags{
Tag: []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
NewAccountObjectIdentifier("tag_name2"),
},
opts.UnsetTag = []ObjectIdentifier{
NewAccountObjectIdentifier("tag_name1"),
NewAccountObjectIdentifier("tag_name2"),
}
assertOptsValidAndSQLEquals(t, opts, `ALTER PIPE %s UNSET TAG "tag_name1", "tag_name2"`, id.FullyQualifiedName())
})
Expand Down
20 changes: 8 additions & 12 deletions pkg/sdk/pipes_validations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,14 @@ func (opts *AlterPipeOptions) validate() error {
if !ValidObjectIdentifier(opts.name) {
errs = append(errs, ErrInvalidObjectIdentifier)
}
if ok := exactlyOneValueSet(opts.Set, opts.Unset, opts.SetTags, opts.UnsetTags, opts.Refresh); !ok {
errs = append(errs, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTags", "UnsetTags", "Refresh"))
if ok := exactlyOneValueSet(
opts.Set,
opts.Unset,
opts.SetTag,
opts.UnsetTag,
opts.Refresh,
); !ok {
errs = append(errs, errExactlyOneOf("AlterPipeOptions", "Set", "Unset", "SetTag", "UnsetTag", "Refresh"))
}
if set := opts.Set; valueSet(set) {
if !anyValueSet(set.ErrorIntegration, set.PipeExecutionPaused, set.Comment) {
Expand All @@ -47,16 +53,6 @@ func (opts *AlterPipeOptions) validate() error {
errs = append(errs, errAtLeastOneOf("AlterPipeOptions.Unset", "PipeExecutionPaused", "Comment"))
}
}
if setTags := opts.SetTags; valueSet(setTags) {
if !valueSet(setTags.Tag) {
errs = append(errs, errNotSet("AlterPipeOptions.SetTags", "Tag"))
}
}
if unsetTags := opts.UnsetTags; valueSet(unsetTags) {
if !valueSet(unsetTags.Tag) {
errs = append(errs, errNotSet("AlterPipeOptions.UnsetTags", "Tag"))
}
}
return errors.Join(errs...)
}

Expand Down