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

feat: serialize AST to json #94

Merged
merged 8 commits into from
Oct 23, 2024
Merged

feat: serialize AST to json #94

merged 8 commits into from
Oct 23, 2024

Conversation

sunng87
Copy link
Member

@sunng87 sunng87 commented Oct 16, 2024

Fixes #93

This patch attempts to add AST serialization to match output of prometheus 3.0 parse_query.

@sunng87 sunng87 linked an issue Oct 16, 2024 that may be closed by this pull request
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 98.63946% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.97%. Comparing base (4ea4cbf) to head (f7aed55).
Report is 1 commits behind head on main.

Changes have been made to critical files, which contain lines commonly executed in production. Learn more

Files with missing lines Patch % Lines
src/parser/function.rs 88.88% 1 Missing ⚠️
src/util/duration.rs 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
- Coverage   98.98%   98.97%   -0.01%     
==========================================
  Files          14       14              
  Lines        6320     6467     +147     
==========================================
+ Hits         6256     6401     +145     
- Misses         64       66       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sunng87 sunng87 marked this pull request as ready for review October 17, 2024 22:46
@sunng87 sunng87 requested a review from waynexia October 17, 2024 22:46
@sunng87
Copy link
Member Author

sunng87 commented Oct 17, 2024

Please let me know if there are promql statements have:

  • subquery that has step is null
  • function that has variadic true

evenyag
evenyag previously approved these changes Oct 22, 2024
Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

Others LGTM. I wonder if we should make some functions pub(crate) only.

src/parser/ast.rs Outdated Show resolved Hide resolved
src/parser/ast.rs Outdated Show resolved Hide resolved
src/util/duration.rs Outdated Show resolved Hide resolved
@sunng87
Copy link
Member Author

sunng87 commented Oct 22, 2024

Sounds good. We should keep minimal access.

@waynexia
Copy link
Member

Please let me know if there are promql statements have:

  • subquery that has step is null
  • function that has variadic true

AFAIK, subquery is a range query that doesn't accept a null step. And existing functions don't have variadic param list

@evenyag evenyag merged commit 7691247 into main Oct 23, 2024
5 checks passed
@evenyag evenyag deleted the feature/serialize-ast branch October 23, 2024 14:25
@sunng87
Copy link
Member Author

sunng87 commented Oct 23, 2024

@waynexia @evenyag could you please cut a release for this?

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.

Serialize to parse_query output
3 participants