Skip to content

Let empty queries export everything #1879

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

Merged
merged 3 commits into from
Sep 16, 2021

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Sep 13, 2021

📔 Description

This changes the empty query, i.e., either a non-provided query argument or an empty string as the query argument, to export everything. Under the hood this is implemented by transforming empty queries into a query that parses everything, namely #type != "this expression matches everything".

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Try it out.

@dominiklohmann dominiklohmann added the feature New functionality label Sep 13, 2021
@dominiklohmann dominiklohmann force-pushed the story/ch28452/empty-query-exports-everything branch from 2631b2f to 0a597b4 Compare September 13, 2021 12:48
Copy link
Member

@mavam mavam left a comment

Choose a reason for hiding this comment

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

Why do you modify the expression parser for this? If the calling context provides the empty string, you can still fill in the somewhat arbitrary expression.

@dominiklohmann
Copy link
Member Author

dominiklohmann commented Sep 13, 2021

Why do you modify the expression parser for this? If the calling context provides the empty string, you can still fill in the somewhat arbitrary expression.

Because that requires touching four pieces of code instead of one, which is much less maintainable. Actually disregard that, I can just do the transformation in read_query. Particularly bad case of the Mondays.

@dominiklohmann dominiklohmann force-pushed the story/ch28452/empty-query-exports-everything branch from 0a597b4 to c68ede4 Compare September 13, 2021 12:56
This changes the empty query, i.e., a non-provided query argument, to
export everything. Under the hood this is implemented by transforming
empty queries into a query that parses everything, namely `#type !=
"this expression matches everything"`.
@dominiklohmann dominiklohmann force-pushed the story/ch28452/empty-query-exports-everything branch from c68ede4 to c628c56 Compare September 13, 2021 14:26
Noticed that we still used `caf::deep_to_string` in queries in log
messages, which really doesn't make sense in user-facing log messages
like "index delays <query> because it is still starting up"; These now
render much more nicely.
@dominiklohmann dominiklohmann requested a review from a team September 13, 2021 14:49
Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

Looks simple enough, and I've tested locally that it works as expected.

It does a bit more than what was discussed in the story; we may want to reconsider if we want to restrict this feature to export since for expose and pivot it is a bit unintuitive, and for vast count a user may expect a more performant implementation for the operation of counting the number of items in the database.

@dominiklohmann dominiklohmann force-pushed the story/ch28452/empty-query-exports-everything branch from ecb9f3a to a3b91d3 Compare September 16, 2021 13:30
Doing this for pivot and explore doesn't really make sense, as that'd
just return everything without any newly added context.
@dominiklohmann dominiklohmann force-pushed the story/ch28452/empty-query-exports-everything branch from a3b91d3 to 76944e1 Compare September 16, 2021 13:32
@dominiklohmann dominiklohmann merged commit 6d55a89 into master Sep 16, 2021
@dominiklohmann dominiklohmann deleted the story/ch28452/empty-query-exports-everything branch September 16, 2021 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants