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

MQE: fix issue where subqueries could return series with no points #9998

Merged
merged 3 commits into from
Nov 25, 2024

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Nov 24, 2024

What this PR does

This PR fixes an issue where MQE could return invalid results if a subquery was evaluated as the top-level expression for an instant query.

If a series had all of its samples filtered out, MQE would return a series with no samples, for example the query (avg(foo) > 100)[10m:30s] could return a result like this:

{
  "status": "success",
  "data": {
    "resultType": "matrix",
    "result": [
      {
        "metric": {
          "env": "prod"
        }
      }
    ]
  }
}

It should instead not return the series at all:

{
  "status": "success",
  "data": {
    "resultType": "matrix",
    "result": [
    ]
  }
}

Which issue(s) this PR fixes or relates to

(none)

Checklist

  • Tests updated.
  • [n/a] Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [n/a] about-versioning.md updated with experimental features.

Copy link
Contributor

@jhesketh jhesketh left a comment

Choose a reason for hiding this comment

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

lgtm

However do we have a test where this happens with a sub-query at not the top of the query. I believe it should be fine, but I think we should have a test for it.

@charleskorn
Copy link
Contributor Author

charleskorn commented Nov 24, 2024

However do we have a test where this happens with a sub-query at not the top of the query. I believe it should be fine, but I think we should have a test for it.

Do you mean something like last_over_time((metric > Inf)[20s:10s]), or something like ((metric > Inf)[20s:10s])[30s:20s]?

@jhesketh
Copy link
Contributor

Do you mean something like last_over_time((metric > Inf)[20s:10s]), or something like ((metric > Inf)[20s:10s])[30s:20s]?

I meant the former, but why not both? ¯_(ツ)_/¯

@charleskorn
Copy link
Contributor Author

Do you mean something like last_over_time((metric > Inf)[20s:10s]), or something like ((metric > Inf)[20s:10s])[30s:20s]?

I meant the former, but why not both? ¯_(ツ)_/¯

Added in 961f2e5

@jhesketh
Copy link
Contributor

Thanks!

@charleskorn charleskorn enabled auto-merge (squash) November 25, 2024 00:04
@charleskorn charleskorn merged commit 1afa9f6 into main Nov 25, 2024
29 checks passed
@charleskorn charleskorn deleted the charleskorn/mqe-empty-range-selectors branch November 25, 2024 00:21
@grafanabot
Copy link
Contributor

The backport to r314 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9998-to-r314 origin/r314
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 1afa9f6750da32c166a55427f0b9cbea227d67da
# Push it to GitHub
git push --set-upstream origin backport-9998-to-r314
git switch main
# Remove the local backport branch
git branch -D backport-9998-to-r314

Then, create a pull request where the base branch is r314 and the compare/head branch is backport-9998-to-r314.

@grafanabot
Copy link
Contributor

The backport to r315 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9998-to-r315 origin/r315
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 1afa9f6750da32c166a55427f0b9cbea227d67da
# Push it to GitHub
git push --set-upstream origin backport-9998-to-r315
git switch main
# Remove the local backport branch
git branch -D backport-9998-to-r315

Then, create a pull request where the base branch is r315 and the compare/head branch is backport-9998-to-r315.

@grafanabot
Copy link
Contributor

The backport to r316 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9998-to-r316 origin/r316
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 1afa9f6750da32c166a55427f0b9cbea227d67da
# Push it to GitHub
git push --set-upstream origin backport-9998-to-r316
git switch main
# Remove the local backport branch
git branch -D backport-9998-to-r316

Then, create a pull request where the base branch is r316 and the compare/head branch is backport-9998-to-r316.

@grafanabot
Copy link
Contributor

The backport to r317 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9998-to-r317 origin/r317
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 1afa9f6750da32c166a55427f0b9cbea227d67da
# Push it to GitHub
git push --set-upstream origin backport-9998-to-r317
git switch main
# Remove the local backport branch
git branch -D backport-9998-to-r317

Then, create a pull request where the base branch is r317 and the compare/head branch is backport-9998-to-r317.

charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998) (#10003)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998) (#10002)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998) (#10001)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 25, 2024
…9998) (#10000)

* MQE: fix issue where subqueries could return series with no points

* Add changelog entry

* Add more test cases

(cherry picked from commit 1afa9f6)

# Conflicts:
#	CHANGELOG.md
@jhesketh jhesketh mentioned this pull request Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants