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

SNOW-1524204: Session Variables passed as connection params are ignored #1173

Closed
OrCh3n opened this issue Jul 8, 2024 · 7 comments
Closed
Assignees
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team

Comments

@OrCh3n
Copy link

OrCh3n commented Jul 8, 2024

  1. What version of GO driver are you using?
    v1.7.2

  2. What operating system and processor architecture are you using?
    darwin/arm64

  3. What version of GO are you using?
    1.21.8

4.Server version:* E.g. 1.90.1
You may get the server version by running a query:
8.24.1

  1. What did you do?
		sfCfg = &gosnowflake.Config{
			<account, user etc.>
			Params: map[string]*string{"$customer_id": lo.ToPtr("test")}
		}
		url, _ = gosnowflake.DSN(sfCfg)
		// open the db using the url
                // execute SHOW VARIABLES -> nothing shows
  1. What did you expect to see?
    as per the docs https://docs.snowflake.com/en/sql-reference/session-variables
    there should be a session variable named CUSTOMER_ID.

The issue is the $ is url encoded, so when parsing the config back from the url it is left url encoded, while it needs to be a $ sign.
ParseDSN should decode the parameter keys IMO.

@OrCh3n OrCh3n added the bug Erroneous or unexpected behaviour label Jul 8, 2024
@github-actions github-actions bot changed the title Session Variables passed as connection params are ignored SNOW-1524204: Session Variables passed as connection params are ignored Jul 8, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Jul 8, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage Issue is under initial triage and removed bug Erroneous or unexpected behaviour labels Jul 8, 2024
@sfc-gh-dszmolka
Copy link
Contributor

hey, thanks for raising this ! so the Params field is supposed to contain only connection and session parameters , hence the name. Please see this doc for reference.

if you're trying to use a session variable , which can be indeed be anything, can you please try specifying them per the documentation you linked; using the SET statement after establishing the connection?
For example, SET customer_id='test'

@sfc-gh-dszmolka sfc-gh-dszmolka added question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team and removed status-triage Issue is under initial triage labels Jul 8, 2024
@sfc-gh-dszmolka
Copy link
Contributor

ah, nevermind, I see now what you're referring to - JDBC is already supporting this notation you mentioned.

@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage Issue is under initial triage enhancement The issue is a request for improvement or a new feature status-triage_done Initial triage done, will be further handled by the driver team and removed question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team status-triage Issue is under initial triage labels Jul 8, 2024
@sfc-gh-dszmolka
Copy link
Contributor

as you mentioned, the session variable indeed comes urlencoded

        "SESSION_PARAMETERS": {
            "%24CUSTOMER_ID": "test",
            "CLIENT_VALIDATE_DEFAULT_PARAMETERS": true
        },

and thus never actually recognized as a session variable, by the server. As a workaround, please consider using the method mentioned previously while we work on this enhancement for the driver to be on par with the JDBC driver regarding this aspect.

@sfc-gh-dszmolka
Copy link
Contributor

hopefully #1177 can address this. Thank you again for pointing this gap out !

@OrCh3n
Copy link
Author

OrCh3n commented Jul 14, 2024

Thanks for the quick response!

@sfc-gh-dszmolka sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels Jul 17, 2024
@sfc-gh-dszmolka
Copy link
Contributor

fix PR is now merged and will be part of the next upcoming release

@sfc-gh-dszmolka
Copy link
Contributor

released with gosnowflake v1.11.0 in July 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

3 participants