Fix correctness issue in predict_linear with step invariant

[harry671003]

Issue

Our continuous correctness tests found an issue with predict_linear with step invariant matrix selector.

Eg: predict_linear({__name__="http_requests_total", pod!~"nginx-1"}[5m] @ start(), -0.37690610678629094)

This PR addresses the problem by allowing the matrixScanner to act in an invariant way similar to Prometheus engine. See: https://github.com/prometheus/prometheus/blob/2a5ed8b8a55fecaa79236ef4adb9f0b82b34587c/promql/engine.go#L1788

[MichaHoffmann]

Can we just wrap the matrix function into step invariant operator? https://github.com/thanos-io/promql-engine/blob/main/execution/step_invariant/step_invariant.go#L47 ?

[MichaHoffmann]

We could just push step inviarnace up in a preprocessor like here: https://github.com/thanos-io/promql-engine/blob/main/logicalplan/plan.go#L335

[harry671003]

We could just push step inviarnace up in a preprocessor like here

Can we just wrap the matrix function into step invariant operator?

The PromQL parser parses the query into:

After calling promql.PreprocessExpr() in plan.go this becomes:

In Prometheus, predict_linear is marked as at modifier unsafe. So it cannot be wrapped with StepInvariance: https://github.com/prometheus/prometheus/blob/308c8c48c15c74a929c430447df3d3c3a3d4001f/promql/functions.go#L1914

[MichaHoffmann]

I see, I wonder if we maybe should jsut fall back for now for correctness sake - this seems to be fairly niche usecase that we must weigh against added complexity

[harry671003]

I see, I wonder if we maybe should jsut fall back for now for correctness sake - this seems to be fairly niche usecase that we must weigh against added complexity

I'm okay with doing the fallback. There is one concern, we'll have to exclude the functions.test file from acceptance tests. If that is okay, I can create a PR.