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.

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

fix: parse dynamic table query from DDL #2438

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 3 additions & 29 deletions pkg/resources/dynamic_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package resources
import (
"context"
"database/sql"
"errors"
"log"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"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/helper/schema"
)

Expand Down Expand Up @@ -243,7 +243,8 @@ func ReadDynamicTable(d *schema.ResourceData, meta interface{}) error {
return err
}

query, err := getQueryFromDDL(dynamicTable.Text)
extractor := snowflake.NewViewSelectStatementExtractor(dynamicTable.Text)
query, err := extractor.ExtractDynamicTable()
if err != nil {
return err
}
Expand All @@ -254,33 +255,6 @@ func ReadDynamicTable(d *schema.ResourceData, meta interface{}) error {
return nil
}

/*
* Previous implementation tried to match query part from the whole dynamic table DDL statement by just using `AS`.
* It was failing for table names containing `AS` (like `REASON`). It was also failing for other parts containing `AS`.
* We cannot simply match by ` AS ` because this can still be part of COMMENT or SELECT query itself.
* We have considered not setting the query at all but it was not ideal because of:
* - possible external changes to dynamic table (drop and recreate externally with different query);
* - import not 100% correct.
* We did not want to complicate the implementation too much by introducing parsers.
* One more thing worth mentioning is the whitespace that can be introduced by the user that is still returned by SHOW.
* For now, we just normalize the DDL before extraction of query.
*
* The outcome implementation matches by ` AS SELECT ` and checks the number of matches.
* If more matches are found, the error is returned to inform user about possible cause of error.
*
* Refer to issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2329.
*/
func getQueryFromDDL(text string) (string, error) {
normalizedDDL := normalizeQuery(text)
matchSubstring := " AS SELECT "
matches := strings.Count(strings.ToUpper(normalizedDDL), matchSubstring)
if matches != 1 {
return "", errors.New("too many matches found. There is no way of getting ONLY the 'query' used to create the dynamic table from Snowflake. We try to get it from the whole creation statement but there may be cases where it fails. Please submit the issue on Github (refer to #2329)")
}
idx := strings.Index(strings.ToUpper(normalizedDDL), " AS SELECT ")
return strings.TrimSpace(normalizedDDL[idx+4:]), nil
}

func parseTargetLag(v interface{}) sdk.TargetLag {
var result sdk.TargetLag
tl := v.([]interface{})[0].(map[string]interface{})
Expand Down
8 changes: 5 additions & 3 deletions pkg/resources/dynamic_table_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"database/sql"
"fmt"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -336,7 +335,7 @@ func TestAcc_DynamicTable_issue2329(t *testing.T) {
func TestAcc_DynamicTable_issue2329_with_matching_comment(t *testing.T) {
dynamicTableName := strings.ToUpper(acctest.RandStringFromCharSet(4, acctest.CharSetAlpha) + "AS" + acctest.RandStringFromCharSet(4, acctest.CharSetAlpha))
tableName := dynamicTableName + "_table"
query := fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)
query := fmt.Sprintf(`with temp as (select "id" from "%v"."%v"."%v") select * from temp`, acc.TestDatabaseName, acc.TestSchemaName, tableName)
m := func() map[string]config.Variable {
return map[string]config.Variable{
"name": config.StringVariable(dynamicTableName),
Expand All @@ -361,7 +360,10 @@ func TestAcc_DynamicTable_issue2329_with_matching_comment(t *testing.T) {
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_DynamicTable_issue2329/1"),
ConfigVariables: m(),
ExpectError: regexp.MustCompile(`too many matches found. There is no way of getting ONLY the 'query' used to create the dynamic table from Snowflake. We try to get it from the whole creation statement but there may be cases where it fails. Please submit the issue on Github \(refer to #2329\)`),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "name", dynamicTableName),
resource.TestCheckResourceAttr("snowflake_dynamic_table.dt", "query", query),
),
},
},
})
Expand Down
53 changes: 52 additions & 1 deletion pkg/snowflake/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,38 @@ func (e *ViewSelectStatementExtractor) ExtractMaterializedView() (string, error)
return string(e.input[e.pos:]), nil
}

func (e *ViewSelectStatementExtractor) ExtractDynamicTable() (string, error) {
fmt.Printf("[DEBUG] extracting dynamic table query %s\n", string(e.input))
e.consumeSpace()
e.consumeToken("create")
e.consumeSpace()
e.consumeToken("or replace")
e.consumeSpace()
e.consumeToken("dynamic table")
e.consumeSpace()
e.consumeID()
// TODO column list
e.consumeSpace()
e.consumeComment()
e.consumeSpace()
e.consumeQuotedParameter("lag")
e.consumeSpace()
e.consumeTokenParameter("warehouse")
e.consumeSpace()
e.consumeTokenParameter("refresh_mode")
e.consumeSpace()
e.consumeTokenParameter("initialize")
e.consumeSpace()
e.consumeTokenParameter("warehouse")
e.consumeSpace()
e.consumeComment()
e.consumeSpace()
e.consumeToken("as")
e.consumeSpace()

return string(e.input[e.pos:]), nil
}

// consumeToken will move e.pos forward iff the token is the next part of the input. Comparison is
// case-insensitive. Will return true if consumed.
func (e *ViewSelectStatementExtractor) consumeToken(t string) bool {
Expand Down Expand Up @@ -134,7 +166,11 @@ func (e *ViewSelectStatementExtractor) consumeNonSpace() {
}

func (e *ViewSelectStatementExtractor) consumeComment() {
if c := e.consumeToken("comment"); !c {
e.consumeQuotedParameter("comment")
}

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

Expand Down Expand Up @@ -173,6 +209,21 @@ func (e *ViewSelectStatementExtractor) consumeComment() {
}
}

func (e *ViewSelectStatementExtractor) consumeTokenParameter(param string) {
if c := e.consumeToken(param); !c {
return
}

e.consumeSpace()

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

e.consumeSpace()
e.consumeNonSpace()
}

func (e *ViewSelectStatementExtractor) consumeClusterBy() {
if e.input[e.pos] != '(' {
return
Expand Down
69 changes: 69 additions & 0 deletions pkg/snowflake/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,75 @@ from bar;`
}
}

func TestViewSelectStatementExtractor_ExtractDynamicTable(t *testing.T) {
basic := "create dynamic table foo lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH as select * from bar;"
caps := "CREATE DYNAMIC TABLE FOO LAG = 'DOWNSTREAM' REFRESH_MODE = 'AUTO' INITIALIZE = 'ON_CREATE' WAREHOUSE = COMPUTE_WH AS SELECT * FROM BAR;"
parens := "create dynamic table foo lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH as (select * from bar);"
multiline := `
create dynamic table foo
lag = 'DOWNSTREAM'
refresh_mode = 'AUTO'
initialize = 'ON_CREATE'
warehouse = COMPUTE_WH
as select *
from bar;`

multilineComment := `
create dynamic table foo
lag = 'DOWNSTREAM'
refresh_mode = 'AUTO'
initialize = 'ON_CREATE'
warehouse = COMPUTE_WH
as
-- comment
select *
from bar;`

comment := `create dynamic table foo lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH comment = 'asdf' as select * from bar;`
commentEscape := `create dynamic table foo lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH comment = 'asdf\'s are fun' as select * from bar;`
orReplace := `create or replace dynamic table foo lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH comment = 'asdf' as select * from bar;`
identifier := `create or replace dynamic table "foo"."bar"."bam" lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH comment = 'asdf\'s are fun' as select * from bar;`
// running SHOW DYNAMIC TABLE in Snowflake actually returns the query with
// the comment before other parameters, even though this is inconsistent
// with the order they are specified in CREATE DYNAMIC TABLE
commentBeforeOtherParams := `create dynamic table foo comment = 'asdf\'s are fun' lag = 'DOWNSTREAM' refresh_mode = 'AUTO' initialize = 'ON_CREATE' warehouse = COMPUTE_WH as select * from bar;`

type args struct {
input string
}
tests := []struct {
name string
args args
want string
wantErr bool
}{
{"basic", args{basic}, "select * from bar;", false},
{"caps", args{caps}, "SELECT * FROM BAR;", false},
{"parens", args{parens}, "(select * from bar);", false},
{"multiline", args{multiline}, "select *\nfrom bar;", false},
{"multilineComment", args{multilineComment}, "-- comment\nselect *\nfrom bar;", false},
{"comment", args{comment}, "select * from bar;", false},
{"commentEscape", args{commentEscape}, "select * from bar;", false},
{"orReplace", args{orReplace}, "select * from bar;", false},
{"identifier", args{identifier}, "select * from bar;", false},
{"commentBeforeOtherParams", args{commentBeforeOtherParams}, "select * from bar;", false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
e := NewViewSelectStatementExtractor(tt.args.input)
got, err := e.ExtractDynamicTable()
if (err != nil) != tt.wantErr {
t.Errorf("ViewSelectStatementExtractor.ExtractDynamicTable() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("ViewSelectStatementExtractor.ExtractDynamicTable() = '%v', want '%v'", got, tt.want)
}
})
}
}

func TestViewSelectStatementExtractor_consumeToken(t *testing.T) {
type fields struct {
input []rune
Expand Down
Loading