snowflakedb/gosnowflake

Gosnowflake unable to use because aws sdk update.

CCGV2 opened this issue ยท 20 comments

gosnowflake unable to use because https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2023-11-17, it changed s3 HeadObjectOutput.ContentLength from int64 to pointer int64, which make anything unable to build/test.

Please answer these questions before submitting your issue.
In order to accurately debug the issue this information is required. Thanks!

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

  2. What operating system and processor architecture are you using?
    MacOS, 13.5.2 (22G91), M1 pro

  3. What version of GO are you using?
    go version go1.21.3 darwin/arm64

4.Server version:* E.g. 1.90.1
not important
5. What did you do?
just testing a unrelated test.

  1. What did you expect to see?
    No error

github.com/snowflakedb/gosnowflake

../../../../go/pkg/mod/github.com/snowflakedb/gosnowflake@v1.7.0/s3_storage_client.go:105:3: cannot use out.ContentLength (variable of type *int64) as int64 value in struct literal

Hi! Same problem here. I use this simple workaround for the moment:

  1. Remove the current dependency requirements
  2. Set the same package versions of gosnowflake v1.7.0 to replace possible updates
  3. Let go mod tidy do the dirty work for you

Package replacements:

replace (
	github.com/aws/aws-sdk-go-v2 => github.com/aws/aws-sdk-go-v2 v1.17.7
	github.com/aws/aws-sdk-go-v2/credentials => github.com/aws/aws-sdk-go-v2/credentials v1.13.18
	github.com/aws/aws-sdk-go-v2/feature/s3/manager => github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.59
	github.com/aws/aws-sdk-go-v2/service/s3 => github.com/aws/aws-sdk-go-v2/service/s3 v1.31.0
)

With that, I'm able to continue to work with the go client for snowflake. There will be a patch for such a problem?

hi and thank you for reporting this issue and for sharing the workaround while the proper fix is in ! we're taking a look and i'll share progress on this thread as events unfold.

--> current status:

fix underway on #971
edit: discussion still ongoing on the approach
edit2: merge held until January 2024 release, as December 2023 supposed to be free of breaking changes (like this would be). Explanation below
edit3: after @kenshaw contribution (thank you!) now it's possible to release the fix without upgrading our dependencies, thus targeting December release

so we're trying to figure out what situation brought this issue for you folks and if anyone could describe the exact steps taken, that would be immensely helpful.

we observed that just by

go get github.com/snowflakedb/gosnowflake@v1.7.0

it doesn't update the AWS SDK.

did any of you by any chance upgrade the AWS SDK apart from the gosnowflake library, e.g. as required by your project ?
if anyone affected by this issue could bring clarity how it got introduced into your project, that would be really helpful, especially for us making a decision how to move forward. Thank you in advance! ๐Ÿ™

did any of you by any chance upgrade the AWS SDK apart from the gosnowflake library, e.g. as required by your project ? if anyone affected by this issue could bring clarity how it got introduced into your project, that would be really helpful, especially for us making a decision how to move forward. Thank you in advance! ๐Ÿ™

In our case, the issue was surfaced when both Snowflake SDK and AWS SDK were updated to the latest.
AWS SDK have broken the compatibility in the latest versions around field nullability (we had to adjust our code base as well).

so we're trying to figure out what situation brought this issue for you folks and if anyone could describe the exact steps taken, that would be immensely helpful.

we observed that just by

go get github.com/snowflakedb/gosnowflake@v1.7.0

it doesn't update the AWS SDK.

did any of you by any chance upgrade the AWS SDK apart from the gosnowflake library, e.g. as required by your project ? if anyone affected by this issue could bring clarity how it got introduced into your project, that would be really helpful, especially for us making a decision how to move forward. Thank you in advance! ๐Ÿ™

In my case, I just made a go get -u to update the project dependencies.

In my case, I just made a go get -u to update the project dependencies.

thank you ! does your project by any chance depend on github.com/aws/aws-sdk-go-v2/feature/s3/manager outside of gosnowflake ?

In my case, I just made a go get -u to update the project dependencies.

thank you ! does your project by any chance depend on github.com/aws/aws-sdk-go-v2/feature/s3/manager outside of gosnowflake ?

No, it doesn't. The main require was github.com/snowflakedb/gosnowflake, I haven't the aws sdk used in the code.

makes sense, thank you! per go help get:

The -u flag instructs get to update modules providing dependencies
of packages named on the command line to use >> newer minor << or patch
releases when available.

which I guess what forced the newer AWS SDK to be downloaded instead of the version specified in the go.mod by gosnowflake (v1.11.59) since AWS decided to release this breaking change with a MINOR version bump only (v1.14.0) instead of MAJOR --> it was allowed by the -u flag probably

this is a very good insight, thank you !

Again thank you all of you who provided your inputs for us to better understand how this breaks in your environment, without gosnowflake upgrading any of the dependencies !

After a long discussion and considering:

  • our upcoming (December) release is supposed to be free from breaking changes
  • upgrading relevant gosnowflake dependencies (#971) would be a breaking change for all of our users and customers, even for those who did not upgrade aws-sdk-go-v2/feature/s3/manager & co and currently not affected

we decided to hold this PR and postpone it into the January release.

We're aware this might not be an ideal situation, but considering all of the factors and the effects, this is what we went with.

For those of you who are affected by AWS releasing the breaking change under a MINOR version change and thus aws-sdk-go-v2/feature/s3/manager v1.14.0 finding its way into your project in some way, please use @MircoT 's workaround.

This thread will be kept updated with the progress, which is unlikely to happen until January release I'm afraid.

Was going to open a separate ticket, but usql will need this issue fixed, as other database drivers that use the AWS SDK will eventually uncover this. The fix is simple:

diff --git a/s3_storage_client.go b/s3_storage_client.go
index 82638e7..c8066de 100644
--- a/s3_storage_client.go
+++ b/s3_storage_client.go
@@ -100,9 +100,13 @@ func (util *snowflakeS3Client) getFileHeader(meta *fileMetadata, filename string
                        out.Metadata[amzMatdesc],
                }
        }
+       var contentLength int64
+       if out.ContentLength != nil {
+               contentLength = *out.ContentLength
+       }
        return &fileHeader{
                out.Metadata[sfcDigest],
-               out.ContentLength,
+               contentLength,
                &encMeta,
        }, nil
 }

I would open a PR for it, but I don't have the ability to run all the unit tests for this driver.

As I realized this wouldn't be backwards compatible, here's a quick version that would work for either version of the SDK:

diff --git a/s3_storage_client.go b/s3_storage_client.go
index 82638e7..f67287c 100644
--- a/s3_storage_client.go
+++ b/s3_storage_client.go
@@ -102,7 +102,7 @@ func (util *snowflakeS3Client) getFileHeader(meta *fileMetadata, filename string
        }
        return &fileHeader{
                out.Metadata[sfcDigest],
-               out.ContentLength,
+               convContentLength(out.ContentLength),
                &encMeta,
        }, nil
 }
@@ -281,3 +281,15 @@ func (util *snowflakeS3Client) getS3Object(meta *fileMetadata, filename string)
                Key:    &s3path,
        }, nil
 }
+
+func convContentLength(v interface{}) int64 {
+       switch x := v.(type) {
+       case int64:
+               return x
+       case *int64:
+               if x != nil {
+                       return *x
+               }
+       }
+       return 0
+}

nice idea, thank you @kenshaw !

so thanks to your above contribution, we can implement and release the fix after all most likely in the December release without upgrading the dependencies - which release should be upcoming in the near future. will keep this thread posted

@sfc-gh-dszmolka Glad this worked for you!

New workaround until a release is cut

// remove when https://github.com/snowflakedb/gosnowflake/issues/970 is resolved
replace github.com/snowflakedb/gosnowflake => github.com/snowflakedb/gosnowflake v1.7.1-0.20231128064355-f33dfc7eb567

That latest workaround causes my build to crash w/o any context (Process finished with the exit code 137 (interrupted by signal 9: SIGKILL)), has anyone else seen this?

I also tried to use the latest version v1.7.1-0.20231205121005-b8e673614933 with the same outcome

Hi @gtomitsuka ! Is there a chance of some repro? All our tests pass either on older version of AWS SDK (master) or after bumping library after the breaking change (https://github.com/snowflakedb/gosnowflake/actions/runs/7111880779)

Nevermind, it's unrelated; I tried with the original fix on v1.7.0 (replace aws-sdk-go-v2 -> ...) and it still didn't work.

released with 1.7.1, closing