Skip to content

Commit

Permalink
feat: Rework data types (#3244)
Browse files Browse the repository at this point in the history
Rework data types handling inside our SDK:
- Extract in a separate package with two main entry points:
- `func ParseDataType(raw string) (DataType, error)` - responsible for
proper datatype parsing and storing the attributes If any (falling to
the hardcoded default for now - this is a continuation from #3020 where
diff suppressions for number and text types were set exactly this way;
for V1 we should stay with hardcoded defaults and we can think of
improving them later on without the direct influence on our users)
- `func AreTheSame(a DataType, b DataType) bool` - responsible for
comparing any two data types; used mostly for diff suppressions and
changes discovery
- Replace all invocations of the old `ToDataType` in a compatible way
(temporary function `LegacyDataTypeFrom` that underneath calls
`ToLegacyDataTypeSql()` which returns the same output as old data types
used two)
- Test on the integration level the consistency of data types' behavior
with Snowflake docs
- Use new parser and comparator in validation and diff suppression
functions (replacing the old ones in all resources)

Next steps/future improvements:
- Use new DataType in SDK SQL builder
- Replace the rest of the old DataType usages (especially in the SDK
Requests/Opts)
- Tackle SNOW-1596962 TODOs regarding VECTOR type
- Improve parsing functions for lists of arguments (and defaults) - will
be done on a case-by-case basis with all the resources
- Clean up the code in new data types (TODOs with SNOW-1843440)
  • Loading branch information
sfc-gh-asawicki authored Dec 5, 2024
1 parent 04f6d54 commit 05ada91
Show file tree
Hide file tree
Showing 54 changed files with 2,627 additions and 640 deletions.
26 changes: 26 additions & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,32 @@ resource "snowflake_tag_association" "table_association" {

The state is migrated automatically. Please adjust your configuration files.

### Data type changes

As part of reworking functions, procedures, and any other resource utilizing Snowflake data types, we adjusted the parsing of data types to be more aligned with Snowflake (according to [docs](https://docs.snowflake.com/en/sql-reference/intro-summary-data-types)).

Affected resources:
- `snowflake_function`
- `snowflake_procedure`
- `snowflake_table`
- `snowflake_external_function`
- `snowflake_masking_policy`
- `snowflake_row_access_policy`
- `snowflake_dynamic_table`
You may encounter non-empty plans in these resources after bumping.

Changes to the previous implementation/limitations:
- `BOOL` is no longer supported; use `BOOLEAN` instead.
- Following the change described [here](#bugfix-handle-data-type-diff-suppression-better-for-text-and-number), comparing and suppressing changes of data types was extended for all other data types with the following rules:
- `CHARACTER`, `CHAR`, `NCHAR` now have the default size set to 1 if not provided (following the [docs](https://docs.snowflake.com/en/sql-reference/data-types-text#char-character-nchar))
- `BINARY` has default size set to 8388608 if not provided (following the [docs](https://docs.snowflake.com/en/sql-reference/data-types-text#binary))
- `TIME` has default precision set to 9 if not provided (following the [docs](https://docs.snowflake.com/en/sql-reference/data-types-datetime#time))
- `TIMESTAMP_LTZ` has default precision set to 9 if not provided (following the [docs](https://docs.snowflake.com/en/sql-reference/data-types-datetime#timestamp)); supported aliases: `TIMESTAMPLTZ`, `TIMESTAMP WITH LOCAL TIME ZONE`.
- `TIMESTAMP_NTZ` has default precision set to 9 if not provided (following the [docs](https://docs.snowflake.com/en/sql-reference/data-types-datetime#timestamp)); supported aliases: `TIMESTAMPNTZ`, `TIMESTAMP WITHOUT TIME ZONE`, `DATETIME`.
- `TIMESTAMP_TZ` has default precision set to 9 if not provided (following the [docs](https://docs.snowflake.com/en/sql-reference/data-types-datetime#timestamp)); supported aliases: `TIMESTAMPTZ`, `TIMESTAMP WITH TIME ZONE`.
- The session-settable `TIMESTAMP` is NOT supported ([docs](https://docs.snowflake.com/en/sql-reference/data-types-datetime#timestamp))
- `VECTOR` type still is limited and will be addressed soon (probably before the release so it will be edited)

## v0.98.0 ➞ v0.99.0

### snowflake_tasks data source changes
Expand Down
3 changes: 2 additions & 1 deletion pkg/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ func DecodeSnowflakeAccountIdentifier(identifier string) (sdk.AccountIdentifier,
}
}

// TODO: use slices.Concat in Go 1.22
// ConcatSlices is a temporary replacement for slices.Concat that will be available after we migrate to Go 1.22.
// TODO [SNOW-1844769]: use slices.Concat
func ConcatSlices[T any](slices ...[]T) []T {
var tmp []T
for _, s := range slices {
Expand Down
24 changes: 23 additions & 1 deletion pkg/resources/diff_suppressions.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ import (
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/datatypes"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

func NormalizeAndCompare[T comparable](normalize func(string) (T, error)) schema.SchemaDiffSuppressFunc {
return NormalizeAndCompareUsingFunc(normalize, func(a, b T) bool { return a == b })
}

func NormalizeAndCompareUsingFunc[T any](normalize func(string) (T, error), compareFunc func(a, b T) bool) schema.SchemaDiffSuppressFunc {
return func(_, oldValue, newValue string, _ *schema.ResourceData) bool {
oldNormalized, err := normalize(oldValue)
if err != nil {
Expand All @@ -22,10 +27,15 @@ func NormalizeAndCompare[T comparable](normalize func(string) (T, error)) schema
if err != nil {
return false
}
return oldNormalized == newNormalized

return compareFunc(oldNormalized, newNormalized)
}
}

// DiffSuppressDataTypes handles data type suppression taking into account data type attributes for each type.
// It falls back to Snowflake defaults for arguments if no arguments were provided for the data type.
var DiffSuppressDataTypes = NormalizeAndCompareUsingFunc(datatypes.ParseDataType, datatypes.AreTheSame)

// NormalizeAndCompareIdentifiersInSet is a diff suppression function that should be used at top-level TypeSet fields that
// hold identifiers to avoid diffs like:
// - "DATABASE"."SCHEMA"."OBJECT"
Expand Down Expand Up @@ -254,3 +264,15 @@ func IgnoreNewEmptyListOrSubfields(ignoredSubfields ...string) schema.SchemaDiff
return len(parts) == 3 && slices.Contains(ignoredSubfields, parts[2]) && new == ""
}
}

func ignoreTrimSpaceSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
return strings.TrimSpace(old) == strings.TrimSpace(new)
}

func ignoreCaseSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
return strings.EqualFold(old, new)
}

func ignoreCaseAndTrimSpaceSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
return strings.EqualFold(strings.TrimSpace(old), strings.TrimSpace(new))
}
12 changes: 6 additions & 6 deletions pkg/resources/external_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (
"strconv"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/datatypes"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -227,11 +227,11 @@ func CreateContextExternalFunction(ctx context.Context, d *schema.ResourceData,
for _, arg := range v.([]interface{}) {
argName := arg.(map[string]interface{})["name"].(string)
argType := arg.(map[string]interface{})["type"].(string)
argDataType, err := sdk.ToDataType(argType)
argDataType, err := datatypes.ParseDataType(argType)
if err != nil {
return diag.FromErr(err)
}
args = append(args, sdk.ExternalFunctionArgumentRequest{ArgName: argName, ArgDataType: argDataType})
args = append(args, sdk.ExternalFunctionArgumentRequest{ArgName: argName, ArgDataType: sdk.LegacyDataTypeFrom(argDataType)})
}
}
argTypes := make([]sdk.DataType, 0, len(args))
Expand All @@ -241,13 +241,13 @@ func CreateContextExternalFunction(ctx context.Context, d *schema.ResourceData,
id := sdk.NewSchemaObjectIdentifierWithArguments(database, schemaName, name, argTypes...)

returnType := d.Get("return_type").(string)
resultDataType, err := sdk.ToDataType(returnType)
resultDataType, err := datatypes.ParseDataType(returnType)
if err != nil {
return diag.FromErr(err)
}
apiIntegration := sdk.NewAccountObjectIdentifier(d.Get("api_integration").(string))
urlOfProxyAndResource := d.Get("url_of_proxy_and_resource").(string)
req := sdk.NewCreateExternalFunctionRequest(id.SchemaObjectId(), resultDataType, &apiIntegration, urlOfProxyAndResource)
req := sdk.NewCreateExternalFunctionRequest(id.SchemaObjectId(), sdk.LegacyDataTypeFrom(resultDataType), &apiIntegration, urlOfProxyAndResource)

// Set optionals
if len(args) > 0 {
Expand Down
5 changes: 3 additions & 2 deletions pkg/resources/external_function_state_upgraders.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/datatypes"
)

type v085ExternalFunctionId struct {
Expand Down Expand Up @@ -52,11 +53,11 @@ func v085ExternalFunctionStateUpgrader(ctx context.Context, rawState map[string]
argDataTypes := make([]sdk.DataType, 0)
if parsedV085ExternalFunctionId.ExternalFunctionArgTypes != "" {
for _, argType := range strings.Split(parsedV085ExternalFunctionId.ExternalFunctionArgTypes, "-") {
argDataType, err := sdk.ToDataType(argType)
argDataType, err := datatypes.ParseDataType(argType)
if err != nil {
return nil, err
}
argDataTypes = append(argDataTypes, argDataType)
argDataTypes = append(argDataTypes, sdk.LegacyDataTypeFrom(argDataType))
}
}

Expand Down
21 changes: 9 additions & 12 deletions pkg/resources/external_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@ import (
"fmt"
"log"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
)

var externalTableSchema = map[string]*schema.Schema{
Expand Down Expand Up @@ -59,11 +56,11 @@ var externalTableSchema = map[string]*schema.Schema{
ForceNew: true,
},
"type": {
Type: schema.TypeString,
Required: true,
Description: "Column type, e.g. VARIANT",
ForceNew: true,
ValidateFunc: IsDataType(),
Type: schema.TypeString,
Required: true,
Description: "Column type, e.g. VARIANT",
ForceNew: true,
ValidateDiagFunc: IsDataTypeValid,
},
"as": {
Type: schema.TypeString,
Expand Down
20 changes: 10 additions & 10 deletions pkg/resources/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import (
"regexp"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/datatypes"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand Down Expand Up @@ -311,7 +311,7 @@ func createScalaFunction(ctx context.Context, d *schema.ResourceData, meta inter
functionDefinition := d.Get("statement").(string)
handler := d.Get("handler").(string)
// create request with required
request := sdk.NewCreateForScalaFunctionRequest(id, returnDataType, handler)
request := sdk.NewCreateForScalaFunctionRequest(id, sdk.LegacyDataTypeFrom(returnDataType), handler)
request.WithFunctionDefinition(functionDefinition)

// Set optionals
Expand Down Expand Up @@ -739,16 +739,16 @@ func parseFunctionArguments(d *schema.ResourceData) ([]sdk.FunctionArgumentReque
if diags != nil {
return nil, diags
}
args = append(args, sdk.FunctionArgumentRequest{ArgName: argName, ArgDataType: argDataType})
args = append(args, sdk.FunctionArgumentRequest{ArgName: argName, ArgDataType: sdk.LegacyDataTypeFrom(argDataType)})
}
}
return args, nil
}

func convertFunctionDataType(s string) (sdk.DataType, diag.Diagnostics) {
dataType, err := sdk.ToDataType(s)
func convertFunctionDataType(s string) (datatypes.DataType, diag.Diagnostics) {
dataType, err := datatypes.ParseDataType(s)
if err != nil {
return dataType, diag.FromErr(err)
return nil, diag.FromErr(err)
}
return dataType, nil
}
Expand All @@ -759,13 +759,13 @@ func convertFunctionColumns(s string) ([]sdk.FunctionColumn, diag.Diagnostics) {
var columns []sdk.FunctionColumn
for _, match := range matches {
if len(match) == 3 {
dataType, err := sdk.ToDataType(match[2])
dataType, err := datatypes.ParseDataType(match[2])
if err != nil {
return nil, diag.FromErr(err)
}
columns = append(columns, sdk.FunctionColumn{
ColumnName: match[1],
ColumnDataType: dataType,
ColumnDataType: sdk.LegacyDataTypeFrom(dataType),
})
}
}
Expand All @@ -789,7 +789,7 @@ func parseFunctionReturnsRequest(s string) (*sdk.FunctionReturnsRequest, diag.Di
if diags != nil {
return nil, diags
}
returns.WithResultDataType(*sdk.NewFunctionReturnsResultDataTypeRequest(returnDataType))
returns.WithResultDataType(*sdk.NewFunctionReturnsResultDataTypeRequest(sdk.LegacyDataTypeFrom(returnDataType)))
}
return returns, nil
}
5 changes: 3 additions & 2 deletions pkg/resources/function_state_upgraders.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/datatypes"
)

type v085FunctionId struct {
Expand Down Expand Up @@ -48,11 +49,11 @@ func v085FunctionIdStateUpgrader(ctx context.Context, rawState map[string]interf

argDataTypes := make([]sdk.DataType, len(parsedV085FunctionId.ArgTypes))
for i, argType := range parsedV085FunctionId.ArgTypes {
argDataType, err := sdk.ToDataType(argType)
argDataType, err := datatypes.ParseDataType(argType)
if err != nil {
return nil, err
}
argDataTypes[i] = argDataType
argDataTypes[i] = sdk.LegacyDataTypeFrom(argDataType)
}

schemaObjectIdentifierWithArguments := sdk.NewSchemaObjectIdentifierWithArgumentsOld(parsedV085FunctionId.DatabaseName, parsedV085FunctionId.SchemaName, parsedV085FunctionId.FunctionName, argDataTypes)
Expand Down
66 changes: 0 additions & 66 deletions pkg/resources/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,79 +3,13 @@ package resources
import (
"fmt"
"slices"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/logging"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

func dataTypeValidateFunc(val interface{}, _ string) (warns []string, errs []error) {
if ok := sdk.IsValidDataType(val.(string)); !ok {
errs = append(errs, fmt.Errorf("%v is not a valid data type", val))
}
return
}

func dataTypeDiffSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
oldDT, err := sdk.ToDataType(old)
if err != nil {
return false
}
newDT, err := sdk.ToDataType(new)
if err != nil {
return false
}
return oldDT == newDT
}

// DataTypeIssue3007DiffSuppressFunc is a temporary solution to handle data type suppression problems.
// Currently, it handles only number and text data types.
// It falls back to Snowflake defaults for arguments if no arguments were provided for the data type.
// TODO [SNOW-1348103 or SNOW-1348106]: visit with functions and procedures rework
func DataTypeIssue3007DiffSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
oldDataType, err := sdk.ToDataType(old)
if err != nil {
return false
}
newDataType, err := sdk.ToDataType(new)
if err != nil {
return false
}
if oldDataType != newDataType {
return false
}
switch v := oldDataType; v {
case sdk.DataTypeNumber:
logging.DebugLogger.Printf("[DEBUG] DataTypeIssue3007DiffSuppressFunc: Handling number data type diff suppression")
oldPrecision, oldScale := sdk.ParseNumberDataTypeRaw(old)
newPrecision, newScale := sdk.ParseNumberDataTypeRaw(new)
return oldPrecision == newPrecision && oldScale == newScale
case sdk.DataTypeVARCHAR:
logging.DebugLogger.Printf("[DEBUG] DataTypeIssue3007DiffSuppressFunc: Handling text data type diff suppression")
oldLength := sdk.ParseVarcharDataTypeRaw(old)
newLength := sdk.ParseVarcharDataTypeRaw(new)
return oldLength == newLength
default:
logging.DebugLogger.Printf("[DEBUG] DataTypeIssue3007DiffSuppressFunc: Diff suppression for %s can't be currently handled", v)
}
return true
}

func ignoreTrimSpaceSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
return strings.TrimSpace(old) == strings.TrimSpace(new)
}

func ignoreCaseSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
return strings.EqualFold(old, new)
}

func ignoreCaseAndTrimSpaceSuppressFunc(_, old, new string, _ *schema.ResourceData) bool {
return strings.EqualFold(strings.TrimSpace(old), strings.TrimSpace(new))
}

func getTagObjectIdentifier(obj map[string]any) sdk.ObjectIdentifier {
database := obj["database"].(string)
schema := obj["schema"].(string)
Expand Down
Loading

0 comments on commit 05ada91

Please sign in to comment.