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: initial implement for promql subquery #2707

Closed
wants to merge 2 commits into from
Closed

Conversation

waynexia
Copy link
Member

@waynexia waynexia commented Nov 7, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Support query like avg by (job) (avg_over_time((up <= 50)[5s:5s]))

This PR only handles range and step in subquery. Other parameters are ignored.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

@waynexia waynexia requested a review from killme2008 November 7, 2023 16:23
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Attention: Patch coverage is 0% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 85.16%. Comparing base (f49cd0c) to head (1404d7a).
Report is 1 commits behind head on main.

❗ Current head 1404d7a differs from pull request most recent head 291133a. Consider uploading reports for the commit 291133a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2707      +/-   ##
==========================================
- Coverage   85.41%   85.16%   -0.25%     
==========================================
  Files         932      763     -169     
  Lines      155075   122920   -32155     
==========================================
- Hits       132451   104690   -27761     
+ Misses      22624    18230    -4394     

Copy link
Contributor

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

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

Good job! But i think we should add some test cases for this feature.

@waynexia
Copy link
Member Author

waynexia commented Nov 9, 2023

Good job! But i think we should add some test cases for this feature.

Test cases in prometheus are a little complicated. I'm going to make a new sqlness interceptor for them.

@waynexia waynexia marked this pull request as draft November 9, 2023 03:04
@killme2008
Copy link
Contributor

@waynexia Don't forget this PR. 👍

@waynexia
Copy link
Member Author

@waynexia Don't forget this PR. 👍

Waiting for CeresDB/sqlness#51

@evenyag evenyag changed the base branch from develop to main December 28, 2023 03:58
@tisonkun
Copy link
Collaborator

@waynexia as you know the sqlness blocker is unblocked. Reminder for this issue :D

@tisonkun
Copy link
Collaborator

tisonkun commented Apr 19, 2024

Close as stale. The changeset of this patch is small. I suppose when we'd like to pick it up, we can open a new PR.

@tisonkun tisonkun closed this Apr 19, 2024
@tisonkun tisonkun deleted the promql-subquery branch April 19, 2024 02:59
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.

3 participants