Skip to content

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Nov 12, 2024

What this PR does

This PR fixes a bug in MQE's implementation of or where it could return incorrect results if both sides contained series with the same labels, and the series on the left side did not have a value for all time steps.

For example, in the expression min(series_1) or min(series_2), both sides have a single series with no labels. Previously, or would have returned two series with the labels {}, rather than correctly returning a single {} series.

I've opted to use DeduplicateAndMerge here, as it's the simplest solution that is already well-tested. A more sophisticated option would be to modify OrBinaryOperation to handle this case itself, but this would come at the cost of increased complexity, and it would largely replicate the behaviour and logic of DeduplicateAndMerge.

The same issue does not affect and or unless, as these operations always return only the left series.

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.

@charleskorn charleskorn marked this pull request as ready for review November 12, 2024 00:44
@charleskorn charleskorn requested a review from a team as a code owner November 12, 2024 00:44
@charleskorn charleskorn force-pushed the charleskorn/mqe-or-bug branch from 5615fa0 to aff7b54 Compare November 12, 2024 00:54
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, nice find

@charleskorn charleskorn enabled auto-merge (squash) November 12, 2024 01:02
@charleskorn charleskorn merged commit b873372 into main Nov 12, 2024
37 checks passed
@charleskorn charleskorn deleted the charleskorn/mqe-or-bug branch November 12, 2024 01:14
@grafanabot
Copy link
Contributor

The backport to r313 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-9874-to-r313 origin/r313
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b873372adbf0996bff70de55934f3dd4a10c7b89
# Push it to GitHub
git push --set-upstream origin backport-9874-to-r313
git switch main
# Remove the local backport branch
git branch -D backport-9874-to-r313

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

@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-9874-to-r314 origin/r314
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b873372adbf0996bff70de55934f3dd4a10c7b89
# Push it to GitHub
git push --set-upstream origin backport-9874-to-r314
git switch main
# Remove the local backport branch
git branch -D backport-9874-to-r314

Then, create a pull request where the base branch is r314 and the compare/head branch is backport-9874-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-9874-to-r315 origin/r315
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b873372adbf0996bff70de55934f3dd4a10c7b89
# Push it to GitHub
git push --set-upstream origin backport-9874-to-r315
git switch main
# Remove the local backport branch
git branch -D backport-9874-to-r315

Then, create a pull request where the base branch is r315 and the compare/head branch is backport-9874-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-9874-to-r316 origin/r316
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b873372adbf0996bff70de55934f3dd4a10c7b89
# Push it to GitHub
git push --set-upstream origin backport-9874-to-r316
git switch main
# Remove the local backport branch
git branch -D backport-9874-to-r316

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

charleskorn added a commit that referenced this pull request Nov 12, 2024
* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 12, 2024
* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 12, 2024
* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 12, 2024
* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

# Conflicts:
#	CHANGELOG.md
charleskorn added a commit that referenced this pull request Nov 12, 2024
* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

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

* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

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

* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

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

* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

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

* MQE: fix `or` where both sides have series with the same labels

* Add changelog entry

# Conflicts:
#	CHANGELOG.md

* Address PR feedback: move `operators.NewDeduplicateAndMerge` to `NewOrBinaryOperation` to make test stronger

(cherry picked from commit b873372)

# 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