Skip to content

Commit

Permalink
Minor fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jmichalak committed Sep 24, 2024
1 parent 2553fbf commit beee8a5
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 16 deletions.
1 change: 0 additions & 1 deletion pkg/datasources/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ func ReadParameters(d *schema.ResourceData, meta interface{}) error {
}
}
parameters, err = client.Parameters.ShowParameters(ctx, &opts)

if err != nil {
return fmt.Errorf("error listing parameters: %w", err)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/sdk/identifier_parsers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sdk
import (
"bytes"
"encoding/csv"
"errors"
"fmt"
"slices"
"strconv"
Expand Down Expand Up @@ -152,6 +153,9 @@ func ParseExternalObjectIdentifier(identifier string) (ExternalObjectIdentifier,

func ParseSchemaObjectIdentifierWithArguments(fullyQualifiedName string) (SchemaObjectIdentifierWithArguments, error) {
splitIdIndex := strings.IndexRune(fullyQualifiedName, '(')
if splitIdIndex == -1 {
return SchemaObjectIdentifierWithArguments{}, errors.New("unable to parse identifier: '(' not present")
}
parts, err := ParseIdentifierString(fullyQualifiedName[:splitIdIndex])
if err != nil {
return SchemaObjectIdentifierWithArguments{}, err
Expand Down
1 change: 1 addition & 0 deletions pkg/sdk/identifier_parsers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ func TestNewSchemaObjectIdentifierWithArgumentsFromFullyQualifiedName_WithRawInp
{RawInput: `abc.def.ghi(FLOAT, VECTOR(INT, 20))`, ExpectedIdentifierStructure: NewSchemaObjectIdentifierWithArguments(`abc`, `def`, `ghi`, DataTypeFloat, "VECTOR(INT, 20)")},
// TODO(SNOW-1571674): Won't work, because of the assumption that identifiers are not containing '(' and ')' parentheses (unfortunately, we're not able to produce meaningful errors for those cases)
{RawInput: `abc."(ef".ghi(FLOAT, VECTOR(INT, 20))`, Error: `unable to read identifier: abc."`},
{RawInput: `abc.def.ghi`, Error: `unable to parse identifier: '(' not present`},
}

for _, testCase := range testCases {
Expand Down
41 changes: 28 additions & 13 deletions pkg/snowflake/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,15 @@ func (e *ViewSelectStatementExtractor) consumeColumn() (isLast bool) {
isLast = true
}
e.consumeSpace()

ok := e.consumeToken("projection policy")
if ok {
e.consumeSpace()
e.consumeID()
if e.input[e.pos-1] == ')' {
isLast = true
e.consumeSpace()
return
}
e.consumeSpace()
}
Expand All @@ -95,13 +98,24 @@ func (e *ViewSelectStatementExtractor) consumeColumn() (isLast bool) {
e.consumeSpace()
e.consumeID()
e.consumeSpace()
e.consumeToken("using")
e.consumeSpace()
e.consumeIdentifierList()
if string(e.input[e.pos-2:e.pos]) == "))" {
isLast = true
if ok := e.consumeToken("using"); ok {
e.consumeSpace()
e.consumeIdentifierList()
if string(e.input[e.pos-3:e.pos-1]) == "))" {
isLast = true
e.consumeSpace()
return
}
e.consumeSpace()
}
e.consumeSpace()
}

e.consumeQuotedParameter("comment", false)
e.consumeSpace()
e.consumeToken(",")
if e.input[e.pos] == ')' {
e.consumeToken(")")
isLast = true
}
return
}
Expand Down Expand Up @@ -201,7 +215,7 @@ func (e *ViewSelectStatementExtractor) ExtractDynamicTable() (string, error) {
e.consumeSpace()
e.consumeComment()
e.consumeSpace()
e.consumeQuotedParameter("lag")
e.consumeQuotedParameter("lag", true)
e.consumeSpace()
e.consumeTokenParameter("warehouse")
e.consumeSpace()
Expand Down Expand Up @@ -266,22 +280,23 @@ func (e *ViewSelectStatementExtractor) consumeNonSpace() {
}

func (e *ViewSelectStatementExtractor) consumeComment() {
e.consumeQuotedParameter("comment")
e.consumeQuotedParameter("comment", true)
}

func (e *ViewSelectStatementExtractor) consumeQuotedParameter(param string) {
func (e *ViewSelectStatementExtractor) consumeQuotedParameter(param string, withEquals bool) {
if c := e.consumeToken(param); !c {
return
}

e.consumeSpace()

if c := e.consumeToken("="); !c {
return
if withEquals {
if c := e.consumeToken("="); !c {
return
}
e.consumeSpace()
}

e.consumeSpace()

if c := e.consumeToken("'"); !c {
return
}
Expand Down
10 changes: 9 additions & 1 deletion pkg/snowflake/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ from bar;`
issue2640 := `CREATE OR REPLACE SECURE VIEW "CLASSIFICATION" comment = 'Classification View of the union of classification tables' AS select * from AB1_SUBSCRIPTION.CLASSIFICATION.CLASSIFICATION union select * from AB2_SUBSCRIPTION.CLASSIFICATION.CLASSIFICATION`
withRowAccessAndAggregationPolicy := `CREATE SECURE VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
withRowAccessAndAggregationPolicyWithEntityKey := `CREATE SECURE VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
allFields := `CREATE OR REPLACE SECURE TEMPORARY VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" (id MASKING POLICY mp USING ("col1", "cond1") PROJECTION POLICY pp COMMENT = 'asdf', foo MASKING POLICY mp USING ("col1", "cond1")) COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
allFieldsListEndingWithMaskingPolicyWithoutUsing := `CREATE OR REPLACE SECURE TEMPORARY VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" (id PROJECTION POLICY pp MASKING POLICY mp COMMENT 'a (s) df', foo MASKING POLICY pp) COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
allFieldsListEndingWithMaskingPolicyWithUsing := `CREATE OR REPLACE SECURE TEMPORARY VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" (id PROJECTION POLICY pp MASKING POLICY mp COMMENT 'a (s) df', foo MASKING POLICY pp USING ("col1")) COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
allFieldsListEndingWithProjectionPolicy := `CREATE OR REPLACE SECURE TEMPORARY VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" (id PROJECTION POLICY pp MASKING POLICY mp COMMENT 'a (s) df' , foo PROJECTION POLICY pp) COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
allFieldsListEndingWithComment := `CREATE OR REPLACE SECURE TEMPORARY VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" (id PROJECTION POLICY pp MASKING POLICY mp COMMENT 'asdf', foo PROJECTION POLICY pp COMMENT 'foo (bar) hoge') COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
allFields := `CREATE OR REPLACE SECURE TEMPORARY VIEW "rgdxfmnfhh"."PUBLIC"."rgdxfmnfhh" (id PROJECTION POLICY pp MASKING POLICY mp USING ("col1", "cond1") COMMENT 'asdf', foo MASKING POLICY mp USING ("col1", "cond1")) COMMENT = 'Terraform test resource' ROW ACCESS policy rap on (title, title2) AGGREGATION POLICY rap ENTITY KEY (foo, bar) AS SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES`
type args struct {
input string
}
Expand Down Expand Up @@ -65,6 +69,10 @@ from bar;`
{"issue2640", args{issue2640}, "select * from AB1_SUBSCRIPTION.CLASSIFICATION.CLASSIFICATION union select * from AB2_SUBSCRIPTION.CLASSIFICATION.CLASSIFICATION", false},
{"with row access policy and aggregation policy", args{withRowAccessAndAggregationPolicy}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
{"with row access policy and aggregation policy with entity key", args{withRowAccessAndAggregationPolicyWithEntityKey}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
{"all fields ending with masking policy without using", args{allFieldsListEndingWithMaskingPolicyWithoutUsing}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
{"all fields ending with masking policy with using", args{allFieldsListEndingWithMaskingPolicyWithUsing}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
{"all fields ending with projection using", args{allFieldsListEndingWithProjectionPolicy}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
{"all fields ending with comment", args{allFieldsListEndingWithComment}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
{"all fields", args{allFields}, "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES", false},
}
for _, tt := range tests {
Expand Down
2 changes: 2 additions & 0 deletions templates/resources/view.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ description: |-

!> **Note about copy_grants** Fields like `is_recursive`, `is_temporary`, `copy_grants` and `statement` can not be ALTERed on Snowflake side (check [docs](https://docs.snowflake.com/en/sql-reference/sql/alter-view)), and a change means recreation of the resource. ForceNew can not be used because it does not preserve grants from `copy_grants`. Beware that even though a change is marked as update, the resource is recreated.

!> Due to Snowflake limitations, to properly compute diff on `statement` field, the provider parses a `text` field which contains the whole CREATE query used to create the resource. We recommend not using special characters, especially `(`, `,`, `)` in any of the fields, if possible.

~> **Required warehouse** For this resource, the provider uses [policy references](https://docs.snowflake.com/en/sql-reference/functions/policy_references) which requires a warehouse in the connection. Please, make sure you have either set a DEFAULT_WAREHOUSE for the user, or specified a warehouse in the provider configuration.

# {{.Name}} ({{.Type}})
Expand Down
Loading

0 comments on commit beee8a5

Please sign in to comment.