snowflakedb / gosnowflake

Go Snowflake Driver
Apache License 2.0
294 stars 122 forks source link

WithStreamDownloader flag causes some errors to be dropped #974

Closed madisonchamberlain closed 8 months ago

madisonchamberlain commented 10 months ago

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? 1.7.0

  2. What operating system and processor architecture are you using? mac m1

  3. What version of GO are you using? run go version in your console go version go1.20.7

5.Server version:* E.g. 1.90.1 NA

I noticed that when the WithStreamDownloader flag is set to true, sometimes there will be no result and no error returned. I am using this with row based result (not arrow based), with result type = queryResultType. I believe this error happens if there is an issue with getting the ChunkDownloader to start. rows.ChunkDownloader.start() returns an error, but no matter what is returned, the error channel gets nil. Is there any reason not to do something like this?

if err := rows.ChunkDownloader.start(); err != nil {
    rows.errChannel <- err
    close(errChannel)
    return err
}
rows.errChannel <- nil // mark query status complete
sfc-gh-dszmolka commented 10 months ago

hi and thank you for raising this issue with us (and for the suggestion as well!) , we're going to take a look

sfc-gh-dszmolka commented 10 months ago

PR under review https://github.com/snowflakedb/gosnowflake/pull/994

madisonchamberlain commented 10 months ago

PR under review #994

Awesome, thank you!

madisonchamberlain commented 10 months ago

I checked the commit from this branch out, and I think it ended up causing more bugs. I am running the query like this

var rows driver.Rows
    var err error
    err = conn.Raw(func(x interface{}) error {
        queryer, implementsQueryContext := x.(driver.QueryerContext)
        if !implementsQueryContext {
            return errors.NewApplication_Internal(ctx, "gosnowflake driver does not implement queryerContext")
        }
        rows, err = queryer.QueryContext(gosnowflake.WithArrowBatches(ctx), "SHOW PARAMETERS LIKE 'TIMEZONE'", nil)
        return err
    })

and now I am seeing this error, which previously was not the case

arrow/ipc: could not read message schema: arrow/ipc: could not read continuation indicator: EOF
sfc-gh-pfus commented 10 months ago

Hello @madisonchamberlain ! I believe this is not related to the stream downloader. It is related to the other fix from yesterday - https://github.com/snowflakedb/gosnowflake/pull/993 The problem here is that you tried to use arrow batches with the query which is not related to data (like show parameters). When you query something like this, Snowflake does not use Arrow as a response format and uses JSON instead. When you ensured that the arrow batches are used, you received the correct error, that previously was just swallowed. Please, just not use WithArrowBatches for this query, and it should work better :)

madisonchamberlain commented 10 months ago

Yes, my mistake I think it was caused by #993 because it worked on sha cf94c15207719b43ac8199d7d183e69b9f70d4ea and stopped working on a4c355786334df0624c003ad6068ff4281cdf975. I will look into a fix for this and file a separate bug, because it seems to break with a lot of queries like select 0 where 1 = 0. I get that the problem is that there is no chunk, but this means queries which return no data will fail in an ambiguous way. Thank you!

sfc-gh-dszmolka commented 8 months ago

released with gosnowflake 1.7.2