Repository: snowflakedb/gosnowflake
Pull request #1177 (2024-07-10 06:39:14)

SNOW-1524204 handle session variables in DSN
Session variables (https://docs.snowflake.com/en/sql-reference/session-variables) must be specified as `$variablename=value` . 

Currently due to their urlencoding, they end up being sent as `%24variablename=value` to the server side, which in turn won't recognize it as a session variable and won't use it.

Fix attempts to introduce a urldecoding function to those custom Params which are session variables to be actually sent as valid session variable format.
Besides the unit test, fix was manually tested to confirm session variables are sent correctly, and a subsequent `SHOW VARIABLES;` query in the same session actually shows the variable.

**edit**: since then, an end-to-end test has been also added; which actually connects to an account, and verifies if the session variables are matching what is sent

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1177?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) Report
Attention: Patch coverage is `71.42857%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 82.98%. Comparing base [(`d5655f3`)](https://app.codecov.io/gh/snowflakedb/gosnowflake/commit/d5655f3732406f6cce29f3913b5e7c4febec2603?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) to head [(`26c6ecc`)](https://app.codecov.io/gh/snowflakedb/gosnowflake/commit/26c6eccee775c99dbea2cae09e9c133e72bd4717?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).

| [Files](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1177?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) | Patch % | Lines |
|---|---|---|
| [dsn.go](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1177?src=pr&el=tree&filepath=dsn.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb#diff-ZHNuLmdv) | 71.42% | [2 Missing :warning: ](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1177?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
- Coverage   83.05%   82.98%   -0.08%     
==========================================
  Files          54       54              
  Lines       12578    12584       +6     
==========================================
- Hits        10447    10443       -4     
- Misses       2131     2141      +10     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1177?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).


Patch:
diff --git a/dsn.go b/dsn.go
index 797ddba..9600af3 100644
--- a/dsn.go
+++ b/dsn.go
@@ -832,7 +832,8 @@ func parseDSNParams(cfg *Config, params string) (err error) {
 			if cfg.Params == nil {
 				cfg.Params = make(map[string]*string)
 			}
-			cfg.Params[param[0]] = &value
+			// handle session variables $variable=value
+			cfg.Params[urlDecodeIfNeeded(param[0])] = &value
 		}
 	}
 	return
@@ -957,3 +958,11 @@ func extractAccountName(rawAccount string) string {
 	}
 	return strings.ToUpper(rawAccount)
 }
+
+func urlDecodeIfNeeded(param string) (decodedParam string) {
+	unescaped, err := url.QueryUnescape(param)
+	if err != nil {
+		return param
+	}
+	return unescaped
+}


Test patch:
diff --git a/dsn_test.go b/dsn_test.go
index 3563434..a7f9103 100644
--- a/dsn_test.go
+++ b/dsn_test.go
@@ -8,6 +8,7 @@ import (
 	cr "crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
+	"database/sql"
 	"encoding/pem"
 	"fmt"
 	"net/url"
@@ -1817,3 +1818,52 @@ func TestExtractAccountName(t *testing.T) {
 		})
 	}
 }
+
+func TestUrlDecodeIfNeeded(t *testing.T) {
+	testcases := map[string]string{
+		"query_tag":             "query_tag",
+		"%24my_custom_variable": "$my_custom_variable",
+	}
+	for param, expected := range testcases {
+		t.Run(param, func(t *testing.T) {
+			decodedParam := urlDecodeIfNeeded(param)
+			assertEqualE(t, decodedParam, expected)
+		})
+	}
+}
+
+func TestUrlDecodeIfNeededE2E(t *testing.T) {
+	customVarName := "CUSTOM_VARIABLE"
+	customVarValue := "test"
+	myQueryTag := "mytag"
+	testPort, err := strconv.Atoi(os.Getenv("SNOWFLAKE_TEST_PORT"))
+	if err != nil {
+		testPort = 443
+	}
+
+	cfg := &Config{
+		Account:  os.Getenv("SNOWFLAKE_TEST_ACCOUNT"),
+		Host:     os.Getenv("SNOWFLAKE_TEST_HOST"),
+		Port:     testPort,
+		Protocol: os.Getenv("SNOWFLAKE_TEST_PROTOCOL"),
+		User:     os.Getenv("SNOWFLAKE_TEST_USER"),
+		Password: os.Getenv("SNOWFLAKE_TEST_PASSWORD"),
+		Params:   map[string]*string{"$" + customVarName: &customVarValue, "query_tag": &myQueryTag},
+	}
+	mydsn, err := DSN(cfg)
+	assertNilE(t, err, "TestUrlDecodeIfNeededE2E failed to create DSN from Config")
+	db, err := sql.Open("snowflake", mydsn)
+	assertNilE(t, err, "TestUrlDecodeIfNeededE2E failed to connect.")
+	defer db.Close()
+	query := "SHOW VARIABLES;"
+	rows, err := db.Query(query)
+	assertNilE(t, err, "TestUrlDecodeIfNeededE2E failed to run SHOW VARIABLES query.")
+	defer rows.Close()
+	var v1, v2, v3, v4, v5, v6, v7 any
+	assertTrueE(t, rows.Next(), "TestUrlDecodeIfNeededE2E query run but no rows were returned.")
+	err = rows.Scan(&v1, &v2, &v3, &v4, &v5, &v6, &v7)
+	assertNilE(t, err, "TestUrlDecodeIfNeededE2E failed to get result.")
+	assertDeepEqualE(t, v4, customVarName, "TestUrlDecodeIfNeededE2E variable name retrieved from the test did not match")
+	assertDeepEqualE(t, v5, customVarValue, "TestUrlDecodeIfNeededE2E variable value retrieved from the test did not match")
+	assertNilE(t, rows.Err(), "TestUrlDecodeIfNeededE2E ERROR getting rows.")
+}