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

WithStreamDownloader flag causes some errors to be dropped #974

Closed
madisonchamberlain opened this issue Nov 21, 2023 · 7 comments
Closed
Assignees
Labels
bug Erroneous or unexpected behaviour

Comments

@madisonchamberlain
Copy link
Contributor

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

@madisonchamberlain madisonchamberlain added the bug Erroneous or unexpected behaviour label Nov 21, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-in_progress Issue is worked on by the driver team label Nov 22, 2023
@sfc-gh-dszmolka
Copy link
Contributor

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-mhofman sfc-gh-mhofman removed the status-in_progress Issue is worked on by the driver team label Dec 5, 2023
@sfc-gh-pfus sfc-gh-pfus removed their assignment Dec 5, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-in_progress Issue is worked on by the driver team label Dec 7, 2023
@sfc-gh-dszmolka
Copy link
Contributor

PR under review #994

@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review and removed status-in_progress Issue is worked on by the driver team labels Dec 7, 2023
@madisonchamberlain
Copy link
Contributor Author

PR under review #994

Awesome, thank you!

@madisonchamberlain
Copy link
Contributor Author

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
Copy link
Collaborator

Hello @madisonchamberlain ! I believe this is not related to the stream downloader. It is related to the other fix from yesterday - #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
Copy link
Contributor Author

Yes, my mistake I think it was caused by #993 because it worked on sha cf94c15 and stopped working on a4c3557. 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 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 Dec 11, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka removed the status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. label Jan 18, 2024
@sfc-gh-dszmolka
Copy link
Contributor

released with gosnowflake 1.7.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Erroneous or unexpected behaviour
Projects
None yet
Development

No branches or pull requests

5 participants