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

Fix with meta flag for analytics queries #561

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Fix with meta flag for analytics queries #561

merged 1 commit into from
Dec 12, 2024

Conversation

Westwooo
Copy link
Contributor

@Westwooo Westwooo commented Dec 6, 2024

Addresses: #556

This change means that the results of an analytics query will not be streamed when the with-meta flag is used, this behaviour will need to be documented. It would be possible to have the meta returned and still stream the queries, but this has two issues:

  1. The output table would be mostly empty as all the rows will have the metadata columns but these would all be empty
  2. The purpose of streaming results is to prevent reading them to memory by streaming to a file, but the format of the metadata means that it cannot be saved to file in a streaming manner.

@Westwooo Westwooo force-pushed the with_meta branch 4 times, most recently from 997e0b5 to 9ef701a Compare December 11, 2024 08:39
src/cli/analytics.rs Outdated Show resolved Hide resolved
inner: vec![],
})?;

let (start, _) = from_utf8(&epilog).unwrap().split_at(epilog.len() - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there no way that we can create the nu value without manually creating a JSON value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@Westwooo Westwooo merged commit 2bdf3ce into main Dec 12, 2024
12 checks passed
@Westwooo Westwooo deleted the with_meta branch December 12, 2024 13:46
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.

2 participants