Release/0.9.3 #176
Release/0.9.3 #176
Conversation
adatzer
force-pushed
the
release/0.9.3
branch
from
85b4e63
to
02e2872
Compare
| ) | ||
|
|
||
| // Specific for Snowflake db | ||
| const ( | ||
| loginTimeout = 5 * time.Second // by default is 60 | ||
| loginTimeout = 5 * time.Second // by default is 60 | ||
| multiStmtName = "multiple statement execution" // https://github.com/snowflakedb/gosnowflake/blob/e909f00ff624a7e60d4f91718f6adc92cbd0d80f/connection.go#L57-L61 |
There was a problem hiding this comment.
Not following the relevance of this comment?
There was a problem hiding this comment.
Just shows where this value comes from. This is what the driver also uses to determine whether it is a multi-statement query.
There was a problem hiding this comment.
Ah ok I think I follow now. So that leads me to the follow-up - in that code it says:
// isMultiStmt returns true if the statement type code is of type multistatement
... so can we not just call isMultiStmt instead of matching strings?
There was a problem hiding this comment.
It would be nicer indeed, but it is unexported name.
There was a problem hiding this comment.
Ah ok ok I follow. Grand so.
| @@ -30,6 +31,7 @@ func (sft SnowFlakeTarget) IsConnectable() bool { | |||
| } | |||
|
|
|||
| func NewSnowflakeTarget(target Target) *SnowFlakeTarget { | |||
| // Note: region connection parameter is deprecated | |||
There was a problem hiding this comment.
// Note: region connection parameter is deprecated
Does it still work? What is to be used in place of it?
There was a problem hiding this comment.
It still works as it is.
The suggestion for the region connection parameter is to be included in :account: (https://godoc.org/github.com/snowflakedb/gosnowflake#hdr-Connection_Parameters). So it has to do with the target configuration in playbooks.
sql_runner/snowflake_target.go
Outdated
| @@ -80,32 +81,42 @@ func (sft SnowFlakeTarget) RunQuery(query ReadyQuery, dryRun bool, showQueryOutp | |||
| return QueryStatus{query, query.Path, 0, nil} | |||
| } | |||
|
|
|||
| scripts := strings.Split(query.Script, ";") | |||
| // The MULTI_STATEMENT_COUNT parameter (through WithMultiStatement) | |||
There was a problem hiding this comment.
I think this comment can be much shorter eg
// 0 allows arbitrary number of statements
adatzer
force-pushed
the
release/0.9.3
branch
from
02e2872
to
507e1e5
Compare
|
Just shortened the comment to |
adatzer
force-pushed
the
release/0.9.3
branch
3 times, most recently
from
7cedbb9
to
2958a1c
Compare
adatzer
force-pushed
the
release/0.9.3
branch
from
2958a1c
to
75f01d9
Compare
adatzer
force-pushed
the
release/0.9.3
branch
from
75f01d9
to
e857de0
Compare
adatzer
force-pushed
the
release/0.9.3
branch
from
6b4abf3
to
15612b6
Compare
adatzer
force-pushed
the
release/0.9.3
branch
from
15612b6
to
528b2ac
Compare
No description provided.