feat: Snowflake adapter using show statements #186

derekbunch · 2025-02-19T02:39:29Z

rebased #109 on master and updated it to use show commands to avoid waking warehouses to prevent extra cost

im not a go dev, but tried my best to follow the patterns i saw as much as possible

derekbunch · 2025-02-19T02:54:53Z

i couldn't figure out a way to do show databases and get the current without 2 queries and that seemed to cause issues, so I left that one in there as I assume it happens much less frequently than structure and columns.

If anybody knows how i could do that, happy to add it.

MattiasMTS · 2025-03-09T16:41:29Z

Hi @derekbunch sorry for slow response, feel free to ping me for faster help :)

Is this ready for review? We are currently focusing on adding somewhat integration tests for existing adapters. Ideally, any new adapter should also try to follow this structure.

We are using testcontainers as the tool for testing each adapter. From quickly checking, it seem there is a localstack image https://hub.docker.com/r/localstack/snowflake (edit: and testcontainer: https://golang.testcontainers.org/modules/localstack/) that you can use to emulate the snowflake warehouse locally. Could you try and take a stab adding this into this PR as well? It would be very much appreciated as it simplifies testing and debugging quite a lot. Let me know if you need any help with this!

derekbunch · 2025-03-10T16:52:30Z

Hi @MattiasMTS, yes it should be ready for review.

And no problem. ill look into the links you shared and try to get those tests implemented for this MR.

MattiasMTS

Nice work! Thanks a lot for the contribution!! ✨
I left some comments - first that should be addressed is the URL part of being able to exclude the snowflake:// protocol in the URL as we are already declaring which driver via the type attribute 😋

MattiasMTS · 2025-03-11T16:07:38Z

dbee/adapters/snowflake_driver.go

+// gosnowflake does not support the snowflake:// scheme
+// , it expects the full connection string excluding the scheme.
+// e.g. "snowflake://user:password@account/db" -> "user:password@account/db"
+type SnowflakeURL struct {


edit: actually, why do we need to care about the snowflake:// protocol prefix here? We declare that a user want to use this adapter via the type input, see e.g.:

{ "url": "user:password@account/db", "id": "snowflake-id-string", "name": "snowflake-name", "type": "snowflake" <- here }

with this in mind, add as docstring to the Connect method that the input URL is expected to be follow the structure as gosnowflake wants it 😄

Or did I miss something here?

~~this doesn't have to be exposed as usage from other packages (i.e. capital initial letter) -> please use snowflakeURL~~

~~feel free to move this to the snowflake.go file as well given this is focused to support the Connect method.~~

~~Finally, you can add use *url.URL inheritance here and ergo don't have to de-reference it in Connect method~~

I realize that I should try and focus on adding some hosted document related to this so it is clear to the users how each adapter works and what underlying driver it is using. Another thing on the TODO list! ✏️

MattiasMTS · 2025-03-11T16:10:28Z

dbee/adapters/snowflake_driver.go

+	`, opts.Schema, opts.Table)
+}
+
+func getSnowflakeStructure(rows core.ResultStream) ([]*core.Structure, error) {


hmm, a bit shame we need to re-do a bit logic here due to the ordering of table, tableType and schema is a bit different. I'll add this to refactoring later on.

MattiasMTS · 2025-03-11T16:12:20Z

dbee/adapters/snowflake_driver.go

+
+		table, tableType, schema := row[1].(string), row[2].(string), row[4].(string)
+
+		if strings.ToLower(schema) == "information_schema" {


just do

Suggested change

if strings.ToLower(schema) == "information_schema" {

if schema == "INFORMATION_SCHEMA" {

or do we expect this to be case insensitive?

MattiasMTS · 2025-03-11T16:14:23Z

dbee/adapters/snowflake_driver.go

+	var structure []*core.Structure
+
+	for k, v := range children {
+		structure = append(structure, &core.Structure{
+			Name:     k,
+			Schema:   k,
+			Type:     core.StructureTypeNone,
+			Children: v,
+		})
+	}


Suggested change

var structure []*core.Structure

for k, v := range children {

structure = append(structure, &core.Structure{

Name: k,

Schema: k,

Type: core.StructureTypeNone,

Children: v,

})

}

structure := make([]*Structure, 0, len(children))

for schema, models := range children {

structure = append(structure, &Structure{

Name: schema,

Schema: schema,

Type: StructureTypeSchema,

Children: models,

})

}

primarily the instantiation of structure to allocate same amount as we have discovered firstly

MattiasMTS · 2025-03-11T16:15:39Z

dbee/adapters/snowflake_driver.go

+	query := `
+    show terse objects;
+  `


reduce some line of code 😋

Suggested change

query := `

show terse objects;

`

query := "show terse objects;"

MattiasMTS · 2025-03-11T17:42:19Z

dbee/adapters/snowflake_test.go

+		wantErr bool
+	}{
+		{
+			name: "Valid URL",


edit: ignore if we decide to not use prefix in URL
feel free to take a look at the example test I gave to the String part - I believe that is the only unit test we need here given this Connect will be tested in integration test. So, let's just add a unit test for the String logic - or wdyt?

MattiasMTS · 2025-03-11T16:32:14Z

dbee/adapters/snowflake.go

+	}, nil
+}
+
+func (r *Snowflake) GetHelpers(opts *core.TableOptions) map[string]string {


take another look at this function please - not sure why the switch statement is needed here 😄

MattiasMTS · 2025-03-11T17:38:17Z

dbee/adapters/snowflake_driver.go

+	url.URL
+}
+
+func (c *SnowflakeURL) String() string {


edit: ignore this, see comment about the URL prefix

~~actually, can't this be simplified by doing:~~

func (s *snowflakeURL) String() string { dsn := s.URL.String() return strings.TrimPrefix(dsn, "snowflake://") }

~~and also add some unit test for this wrapping logic:~~

func TestSnowflakeURL_String(t *testing.T) { tests := []struct { name string input string want string }{ { name: "should return dsn without snowflake prefix", input: "snowflake://user:pass@myaccount/mydb", want: "user:pass@myaccount/mydb", }, { name: "should return input string if snowflake prefix not present", input: "user:pass@myaccount/mydb?warehouse=compute", want: "user:pass@myaccount/mydb?warehouse=compute", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { parsed, err := url.Parse(tt.input) require.NoError(t, err) surl := snowflakeURL{parsed} got := surl.String() require.Equal(t, tt.want, got) }) } }

MattiasMTS · 2025-03-11T17:40:20Z

dbee/adapters/snowflake_test.go

+				r.Error(err)
+				return
+			}
+			r.NoError(err)


the want here is never checked. Missing require.Equal(t, tt.want, got), no need to use a require.New here :)

edit: feel free to ignore this if you're going to just add the String method :)

MattiasMTS · 2025-03-11T17:40:46Z

dbee/adapters/snowflake_test.go

+			if (err != nil) != tt.wantErr {
+				t.Errorf("NewSnowflake() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}


I think this can be skipped, or what are you trying to check here?
edit: feel free to ignore this if you're going to just add the String method :)

derekbunch · 2025-03-22T06:04:33Z

Hi @derekbunch sorry for slow response, feel free to ping me for faster help :)

Is this ready for review? We are currently focusing on adding somewhat integration tests for existing adapters. Ideally, any new adapter should also try to follow this structure.

We are using testcontainers as the tool for testing each adapter. From quickly checking, it seem there is a localstack image https://hub.docker.com/r/localstack/snowflake (edit: and testcontainer: https://golang.testcontainers.org/modules/localstack/) that you can use to emulate the snowflake warehouse locally. Could you try and take a stab adding this into this PR as well? It would be very much appreciated as it simplifies testing and debugging quite a lot. Let me know if you need any help with this!

@MattiasMTS I've looked into this a bit and tried to get it set up. Looks like it needs an auth token to run the container so I created an account, generated an auth token, and tried to run the container with it and it gives me this error

❯docker run \
    --rm -it \
    -p 127.0.0.1:4566:4566 \
    -p 127.0.0.1:4510-4559:4510-4559 \
    -p 127.0.0.1:443:443 \
    -e LOCALSTACK_AUTH_TOKEN="<redacted>" \
    localstack/snowflake
                                                                                                                                                        
LocalStack version: 4.2.1.dev76
LocalStack build date: 2025-03-20
LocalStack build git hash: 6b2781950

Docker not available
2025-03-22T05:51:45.089  INFO --- [  MainThread] l.p.c.b.licensingv2        : Successfully requested and activated new license <redacted>:hobby 🔑✅
2025-03-22T05:51:47.017  INFO --- [  MainThread] l.p.c.extensions.platform  : loaded 1 extensions
⚠ The Snowflake emulator is currently not covered by your license. ❄ Please reach out to LocalStack Support to get access to this product. ⚠

Any idea how i should proceed? Do y'all use a ci token for your ci where this would run or is this a non-starter?

MattiasMTS · 2025-03-23T13:45:04Z

Any idea how i should proceed? Do y'all use a ci token for your ci where this would run or is this a non-starter?

ahh, right. I guess that version of localstack requires a subscription.. That is a bit unfortunate. Feel free to skip this part/scope of this PR then and I'll try and find if we can get a sub based on OSS stuff. Thanks a lot for trying it out and feedback the info! 😄

MattiasMTS · 2025-03-23T13:46:12Z

dbee/adapters/snowflake.go

+	err = db.Ping()
+	if err != nil {
+		return nil, err
+	}


let's ping with a timeout here, e.g

ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() if err := db.PingContext(ctx); err != nil { return nil, fmt.Errorf("unable to ping snowflake: %w", err) }

if you want to ping it

😮 super cool, thanks! didnt know this is how you did that!

MattiasMTS · 2025-03-23T13:48:05Z

dbee/adapters/snowflake_driver.go

+	config.Database = name
+	connector := gosnowflake.NewConnector(gosnowflake.SnowflakeDriver{}, config)
+	db := sql.OpenDB(connector)
+	err := db.Ping()


same comment here on ping as in the connector 😋

MattiasMTS · 2025-03-23T13:48:39Z

dbee/adapters/snowflake_test.go

+	"github.com/kndndrj/nvim-dbee/dbee/core/builders"
+)
+
+func TestNewSnowflake(t *testing.T) {


I think you can remove this test since I am not sure we need it for testing anymore?

ah yes, was just waiting to delete this until i confirmed how to handle testing!

MattiasMTS · 2025-03-23T13:49:54Z

dbee/adapters/snowflake_driver.go

+// snowflakeDriver is a sql client for snowflakeDriver.
+type snowflakeDriver struct {
+	c      *builders.Client
+	config *gosnowflake.Config


feel free to make this a non-pointer if you are only using it in SelectDatabase with dereference, or wdyt?

MattiasMTS · 2025-03-23T13:50:35Z

dbee/adapters/snowflake.go

+// or
+// host:port/database/schema?account=user_account[?param1=value1&paramN=valueN]
+// https://github.com/snowflakedb/gosnowflake/blob/b034584aa6fc171c1fa02e5af1f98234f24538fe/dsn.go#L308-#L314
+func (r *Snowflake) Connect(rawURL string) (core.Driver, error) {


looks great! We can iteratore more on what kind of connection inputs a user can have here later on 😄 This looks a lot cleaner now IMO! ✨

yea, im also planning to update this (if needed) to support oauth once snowflake merges this branch since that or 2FA is snowflake's preferred method of auth. Password based auth will be disabled soon

before i knew about this branch, i tried implementing oauth myself, but seemed like there were a number of issues with their TokenAccessor implementation, so i just opted to wait for them to work out the kinks in their branch, then update this to use it once its ready

- Implement comprehensive Snowflake database adapter - Support for password, keypair (JWT), and MFA/SSO authentication methods - Private key loading with PKCS1/PKCS8 format support and encryption - Cost-optimized queries using SHOW statements to avoid warehouse wake-up - Database operations: structure browsing, column metadata, database switching - Complete unit test suite with authentication method validation - Documentation with setup guide and troubleshooting - Environment variable template support for secure credential management Addresses: kndndrj#23, builds on kndndrj#109 and kndndrj#186

derekbunch added 5 commits February 18, 2025 19:33

snowflake branch base copy

701f000

updateing to use show instead of select, not working yet

36eb13f

columns and structure

3a2b15c

show transient databases

f446019

go mod tidy

d1b9c03

derekbunch changed the title ~~Snowflake adapter using show statements~~ feat: Snowflake adapter using show statements Feb 20, 2025

MattiasMTS reviewed Mar 11, 2025

View reviewed changes

address PR comments

80bf952

MattiasMTS reviewed Mar 23, 2025

View reviewed changes

address PR comments

0bc71c2


		table, tableType, schema := row[1].(string), row[2].(string), row[4].(string)

		if strings.ToLower(schema) == "information_schema" {

	if strings.ToLower(schema) == "information_schema" {
	if schema == "INFORMATION_SCHEMA" {

feat: Snowflake adapter using show statements #186

Are you sure you want to change the base?

feat: Snowflake adapter using show statements #186

Conversation

derekbunch commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

derekbunch commented Feb 19, 2025

Uh oh!

MattiasMTS commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

derekbunch commented Mar 10, 2025

Uh oh!

MattiasMTS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MattiasMTS Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekbunch commented Mar 22, 2025

Uh oh!

MattiasMTS commented Mar 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekbunch Mar 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

derekbunch commented Feb 19, 2025 •

edited

Loading

MattiasMTS commented Mar 9, 2025 •

edited

Loading

MattiasMTS Mar 11, 2025 •

edited

Loading

derekbunch Mar 29, 2025 •

edited

Loading