Skip to content

feat: add support for fill modifiers in binary operations#62

Merged
f41gh7 merged 3 commits into
VictoriaMetrics:masterfrom
andriibeee:fill-modifiers
Jul 8, 2026
Merged

feat: add support for fill modifiers in binary operations#62
f41gh7 merged 3 commits into
VictoriaMetrics:masterfrom
andriibeee:fill-modifiers

Conversation

@andriibeee

@andriibeee andriibeee commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

part of VictoriaMetrics/VictoriaMetrics#10598

Summary by cubic

Added support for fill modifiers (fill, fill_left, fill_right) in binary operations to set default values when one side is missing. This expands query syntax and improves control over join behavior.

  • New Features
    • Parser recognizes fill, fill_left, and fill_right modifiers with numeric values, including negatives, inf, and NaN.
    • fill(x) sets both FillLeft and FillRight; fill_left(x) and fill_right(x) set sides independently.
    • Works alongside existing on()/group_* modifiers and is treated as a reserved identifier.
    • Output formatting includes the fill modifiers via appendModifiers.
    • Added tests for case-insensitive syntax, value parsing, and normalization.

Written for commit 14411c8. Summary will update on new commits.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 3 files

@codecov

codecov Bot commented Mar 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.57%. Comparing base (0dc0e2d) to head (1718c1f).
⚠️ Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
parser.go 77.41% 7 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
- Coverage   89.69%   89.57%   -0.13%     
==========================================
  Files          11       11              
  Lines        2969     3069     +100     
==========================================
+ Hits         2663     2749      +86     
- Misses        209      216       +7     
- Partials       97      104       +7     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Haleygo Haleygo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the pull request, it looks good!

Just two things:

  1. the fill modifiers should not work with and, or, unless (existing operators in PromQL), since the whole point of those operators is to filter series based on their presence or absence, nor with if, ifnot, default (new operators in MetricsQL).
    So in parseExpr(), the check

    metricsql/binary_op.go

    Lines 71 to 74 in a808ea4

    func isBinaryOp(op string) bool {
    op = strings.ToLower(op)
    return binaryOps[op]
    }
    is not sufficient to determine whether parseFillModifier is legitimate , a new sub-op check seems needed.

  2. the optimizer check for binary expression must also consider fill modifier.

Update:
pushed above change in 02e980d,
1718c1f

@f41gh7 f41gh7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@f41gh7
f41gh7 merged commit 9ec5396 into VictoriaMetrics:master Jul 8, 2026
2 of 4 checks passed
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.

4 participants