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

adjusted handling of zero-rows results #121

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Conversation

gabor
Copy link
Contributor

@gabor gabor commented May 22, 2024

( fixes #118 )

when the multi format was added, we unintentionally changed how we handle zero-rows sql-responses.. previously, for all-formats-except-timeseries, we returned the real dataframe-fields (the fields had zero items in them).

this PR adjusts the behavior to be the same as it was before. (i literally took the dataframe_test.go file, and ran it on a git-checkout of the old-version, and wrote down the results there).

NOTE: there was no format=multi in the "before" situation, so for that case, i kept the current behavior (no dataframe-fields when zero-rows).

@gabor gabor requested a review from a team as a code owner May 22, 2024 14:24

frame.Meta.ExecutedQueryString = query.RawSQL
frame.Meta.PreferredVisualization = data.VisTypeGraph

switch query.Format {
case FormatOptionMulti:
if zeroRows {
return nil, ErrorNoResults
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have to return an error? or should we just return an empty frame with a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not cause an error "in the browser". this is only used in the go-code, and eventually it reaches here:

sqlds/datasource.go

Lines 249 to 251 in 6631da7

if errors.Is(err, ErrorNoResults) {
return res, nil
}

so no error is returned. (the dataframe at that point is a more&less empty one, containing the executed-query-string).

personally i'd love to adjust the code to not have this "marker" error, but not sure what others think. also wanted to minimize the size of the diff.
WDYT?

@gabor gabor requested a review from scottlepp May 29, 2024 09:14
@gabor gabor merged commit 7ef7f76 into main Jun 3, 2024
3 checks passed
@gabor gabor deleted the gabor/zero-results-fixes branch June 3, 2024 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add back frame.fields for no-rows table-queries
2 participants