feat(promql): add translator phase for vector matching - #155634
Conversation
de7fb3f to
12241fe
Compare
12241fe to
371b088
Compare
dd60cbd to
5bdd6c7
Compare
ba400cd to
8e5572d
Compare
ec73fb4 to
b35a1e0
Compare
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
7bd590d to
72fecba
Compare
|
@sidosera I took a quick look - the approach seems reasonable. Cursor found two queries that don't work correctly. Can you double check? I'll review it in detail tomorrow. Sorry for the delay. |
|
I took another look, and the PR looks good. Can you confirm the two issues above? AI found another case where this might not work correctly. Can you also double-check this one? |
That is related to #158164. The other two cases you highlighted look legit. Since I had to fix those I also fixed the first one and stacked this PR atop of it #158174. PTAL. |
ca2356f to
b82df37
Compare
399fa05 to
ed529c4
Compare
1c5837b to
fad1b79
Compare
ed529c4 to
4e9c01e
Compare
Translate binary operators with explicit vector matching - on(...), ignoring(...), group_left, group_right - and the operators composed over their results as an inner join: each operand compiles as its own series pipeline against the labels the join requires, the two are joined on step plus the packed match key, and the result value is computed on the joined rows. A label the match dropped that an enclosing translation still requires null-fills at the join. The verifier admits vector matching behind the PROMQL_VECTOR_MATCHING_V0 capability, requires concrete label sets on both operands, and keeps rejecting default matching between mismatched label sets (#158374). InnerJoin counts towards the PROMQL telemetry metric, and the unmapped_fields check reports one failure per command rather than one per pipeline. The join block is laid out by VectorBinaryOperatorLayout, constructed from the operator: the translator translates the two operands and the layout decides orientation, match keys, the InnerJoin, the null-filled output labels, the value expression and filter mode, and the final projection. The build operand is re-identified and its value column renamed because InnerJoin merges its output by name.
The vector-match join was laid out by a VectorBinaryOperatorLayout class of its own. It had one caller, and reading the translator meant chasing orientation, keys, join and output into another file. Bring it back as translator methods next to doTranslateBinOpInnerJoin: emitJoin, emitJoinInput, joinKey, bindJoinOutput, bindJoinResult, reidentify. No behavior change.
Move the join layout out of the translator into VectorBinaryOperatorLayout, a fluent builder that owns the three rules of the block: how the sides are ordered (probe vs build), how the fields are placed (packed match keys, carried build columns, null-filled labels) and what is projected. The translator translates the operands and feeds them in. Assert the Filter.NONE case explicitly: without on/ignoring the verifier only admits operands with concrete label sets, so the header needs no widening.
The join key packed each operand's own label columns in the order that operand declared them, and DimsPacker encodes values by position. Two operands grouping by the same labels in a different order therefore never matched: `sum by (cluster, pod) (a) / ignoring () sum by (pod, cluster) (b)` returned no rows. Both sides now pack the same label list: the on(...) labels as written, otherwise the sorted union of both operands' labels minus the ignored ones, with a null where a side lacks a label. That is a Prometheus signature: a label absent on both sides does not discriminate, a label present on one side only never matches.
`sum by (cluster) (a) + sum by (pod) (b)` was rejected by the verifier as "mismatched grouping keys". Prometheus evaluates it: each pair is compared on its actual label set, which for different concrete label sets never coincides, so the result is the empty vector carrying the left operand's labels. Route such operators to the join, whose shared-order key now implements exactly that, and declare the left labels as the output. The rejection remains only where vector matching is not enabled.
4e9c01e to
f83392a
Compare
Elasticsearch gained PromQL vector matching on Serverless and Stack 9.6 (elastic/elasticsearch#155634), but `_PROMQL_UNSUPPORTED_RE` still listed `on(`, `ignoring(`, `group_left`, and `group_right` among the constructs blocked unconditionally. That list predates the capability, so every matched panel fell through to the ES|QL join/ratio approximation no matter what the target could actually evaluate — losing labels and warning about it on clusters that could have run the operator's PromQL verbatim. Going native needs two independent facts, and guessing either one is expensive: emitting a query the target cannot plan replaces a rendering panel with a hard Kibana error, which is strictly worse than an approximation. So both are established rather than assumed. The target half is probed, not version-gated, because Serverless carries the capability without a 9.6 `version.number`. The probe matches `vector(1)` operands so it needs no index or data, and it fails closed: only HTTP 200 enables the native path, so an older stack, an auth error, or an unreachable cluster all keep today's behavior. The shape half exists because support is necessary but not sufficient — Elasticsearch only matches operands whose label set it can determine statically, and rejects anything else with `vector matching requires operands with concrete label sets`. A new AST predicate models that rule, so a raw selector, a `rate()`/`*_over_time()` over one, or `without (...)` keeps the ES|QL translation. This is why the issue's own `group_left` example over bare selectors stays on ES|QL: emitting it natively would break a working panel. Panels that stay behind say which of the two halves declined, distinguishing an unverifiable probe from a verified absence so the operator knows whether to upgrade, fix access, or reshape the query. Panels that go native record `minimum_kibana_version: 9.6.0`, because the artifact is portable while the query it carries is not. Comment handling is part of the gate rather than an afterthought: PromQL allows a comment between a token and its parenthesis, so `on # note\n(x)` would otherwise hide a matcher from the check and route it native on a target that cannot run it. Comments are stripped from the raw expression, where their extent is exact; the gates that scan `_clean_promql_for_native` output keep scanning it with comments intact, because flattening newlines destroys that extent and stripping there would swallow real operands. `or`/`and`/`unless` with a matcher stay blocked (elastic/elasticsearch#158181). The verifier's label-set classifier now also recognizes the 9.6 wording, so a shape that ever slips past this gate is reported as a translator bug instead of going unlabeled.
What
Translates binary operators with explicit vector matching -
on(...),ignoring(...),group_left,group_right- and the operators composed over their results as an inner join. Stacked on #158430, which carries the translator model this builds on; this PR is the vector-matching addition only.How
JoinLayout); the two are joined onstepplus the packed match key, and the result value is computed on the joined rows. A label the match dropped that an enclosing translation still requires null-fills at the join.PROMQL_VECTOR_MATCHING_V0, requires concrete label sets on both operands, and keeps rejecting default matching between mismatched label sets (#158374).InnerJoincounts towards the PROMQL telemetry metric; theunmapped_fieldscheck reports one failure per command rather than one per pipeline.Testing
k8s-timeseries-promql-vm.csv-spec,PromqlVectorMatchingIT,PromqlPlanBinaryOperatorTests,PromqlVerifierTests.-> 88%