Problem
asap-planner-rs/src/planner/elastic_dsl.rs:126-129 constructs
KeyByLabelNames via struct literal, bypassing KeyByLabelNames::new():
(
KeyByLabelNames { labels: group_fields },
KeyByLabelNames { labels: rollup },
)group_fields is the Elastic DSL query's own group_by clause order
(whatever the user wrote), and rollup comes from an IndexSet difference —
neither is sorted. Everywhere else in the codebase, KeyByLabelNames::new()
unconditionally sorts label names, so this is the one construction path that
produces non-canonical label-name order.
Today this is safe: Elastic DSL has no binary-arithmetic-expression concept,
and a SimpleEngine instance is permanently locked to one QueryLanguage at
construction (simple_engine/mod.rs:1040-1044), so Elastic-planned
AggregationConfigs never reach a binary-expr join, positional or otherwise.
If Elastic DSL ever gains binary arm support (+/-/*// between two
Elastic queries), whatever join code serves it would need to either route
these labels through KeyByLabelNames::new() first, or be written to key
on canonical (name, value) pairs rather than positional order — the same
class of bug investigated and ruled out for PromQL in #567.
Found via
Investigating a (ultimately nonexistent) label-order join bug for #567.
Problem
asap-planner-rs/src/planner/elastic_dsl.rs:126-129constructsKeyByLabelNamesvia struct literal, bypassingKeyByLabelNames::new():group_fieldsis the Elastic DSL query's owngroup_byclause order(whatever the user wrote), and
rollupcomes from anIndexSetdifference —neither is sorted. Everywhere else in the codebase,
KeyByLabelNames::new()unconditionally sorts label names, so this is the one construction path that
produces non-canonical label-name order.
Today this is safe: Elastic DSL has no binary-arithmetic-expression concept,
and a
SimpleEngineinstance is permanently locked to oneQueryLanguageatconstruction (
simple_engine/mod.rs:1040-1044), so Elastic-plannedAggregationConfigs never reach a binary-expr join, positional or otherwise.If Elastic DSL ever gains binary arm support (
+/-/*//between twoElastic queries), whatever join code serves it would need to either route
these labels through
KeyByLabelNames::new()first, or be written to keyon canonical (name, value) pairs rather than positional order — the same
class of bug investigated and ruled out for PromQL in #567.
Found via
Investigating a (ultimately nonexistent) label-order join bug for #567.