Summary
While auditing QueryExpr node names for #183, found that #205's fold of the old, separate Expr<C> scalar-expression tree into QueryExpr itself left behind pairs of variants that represent the same semantic value/operation, differing only in which tree position they're allowed to appear in — a relational/vector-tree position (pre-existing QueryExpr variants) versus a scalar-sub-language position (the variants folded in by #205).
Instance 1: Scalar(f64) vs Literal(ScalarValue)
QueryExpr::Scalar(f64) — a PromQL bare numeric literal / folded constant. Appears only as a BinaryOp operand (<vector> op <scalar>, e.g. up > 1).QueryExpr::Literal(ScalarValue) — a typed scalar constant (Int64/Float64/Utf8/Boolean/Null), used inside the scalar-expression sub-language (Compare/Arith/Case/InList operands, etc.).
Every PromQL scalar value is a f64, so Literal(ScalarValue::Float64(v)) holds exactly the same value Scalar(v) does. The two variants exist side by side with no semantic difference in the value they carry — only in which part of the tree constructs/expects them.
Note from the investigation: output_schema (crates/types/src/pre_asap/query_expr.rs, ~line 1100 and ~1133-1154), canonicalize.rs (~line 64-85), and resolve.rs (~line 87, 348) all currently use the variant tag itself to distinguish "a scalar-bridge leaf that has its own row schema" (Scalar/EvalTime/VectorFromScalar/ScalarFromVector) from "a nested scalar sub-expression with no row schema of its own" (Literal/Column/Compare/…, which return QueryExprError::ScalarHasNoRowSchema). That distinction currently rides entirely on which of these two variants is used.
Instance 2: BinaryOp vs Compare/Arith
QueryExpr::BinaryOp{op: BinaryOpKind, lhs, rhs, vector_match} — the relational/vector-tree binary operator node (PromQL binary ops between two vector-shaped QueryExprs, or a vector and a scalar bridge).QueryExpr::Compare{left, op: CompareOp, right} and QueryExpr::Arith{op: ArithOp, left, right} — the scalar-expression sub-language's own binary comparison/arithmetic nodes (used inside Filter.pred, Aggregate.having, Case branches, etc.).
BinaryOpKind literally reuses ArithOp/CompareOp internally (BinaryOpKind::Arith(ArithOp) / BinaryOpKind::Compare(CompareOp), see the doc comment on BinaryOpKind in query_expr.rs) — so the same operator vocabulary is expressed through two different node shapes depending on which side of the relational/scalar split the expression sits on.
Same discriminant reliance as Instance 1: canonicalize.rs:83 gives BinaryOp{lhs,rhs} its own recursion arm (vec![lhs.as_mut(), rhs.as_mut()]) while Compare/Arith (canonicalize.rs:86,95) fall into the generic scalar-leaf bucket alongside Literal; resolve.rs:320,335 gives BinaryOp its own real match arm while Compare/Arith (resolve.rs:349,358) sit in that module's catch-all too. Three separate functions currently tell "relational, has real children to recurse into" from "nested scalar leaf" purely by which of these two variants was used.
Why this is worth tracking
Both instances follow the same shape: a relational-tree node and a scalar-sub-language node exist in parallel for what is, value/operator-wise, the same thing — an artifact of #205 unifying the type (Expr<C> folded into QueryExpr<C>) without unifying the shapes that had accumulated on both sides of the original relational/scalar split. Filing this purely to record the observation from the #183/#184 audit; no proposed resolution here.
Summary
While auditing
QueryExprnode names for #183, found that #205's fold of the old, separateExpr<C>scalar-expression tree intoQueryExpritself left behind pairs of variants that represent the same semantic value/operation, differing only in which tree position they're allowed to appear in — a relational/vector-tree position (pre-existingQueryExprvariants) versus a scalar-sub-language position (the variants folded in by #205).Instance 1:
Scalar(f64)vsLiteral(ScalarValue)QueryExpr::Scalar(f64)— a PromQL bare numeric literal / folded constant. Appears only as aBinaryOpoperand (<vector> op <scalar>, e.g.up > 1).QueryExpr::Literal(ScalarValue)— a typed scalar constant (Int64/Float64/Utf8/Boolean/Null), used inside the scalar-expression sub-language (Compare/Arith/Case/InListoperands, etc.).Every PromQL scalar value is a
f64, soLiteral(ScalarValue::Float64(v))holds exactly the same valueScalar(v)does. The two variants exist side by side with no semantic difference in the value they carry — only in which part of the tree constructs/expects them.Note from the investigation:
output_schema(crates/types/src/pre_asap/query_expr.rs, ~line 1100 and ~1133-1154),canonicalize.rs(~line 64-85), andresolve.rs(~line 87, 348) all currently use the variant tag itself to distinguish "a scalar-bridge leaf that has its own row schema" (Scalar/EvalTime/VectorFromScalar/ScalarFromVector) from "a nested scalar sub-expression with no row schema of its own" (Literal/Column/Compare/…, which returnQueryExprError::ScalarHasNoRowSchema). That distinction currently rides entirely on which of these two variants is used.Instance 2:
BinaryOpvsCompare/ArithQueryExpr::BinaryOp{op: BinaryOpKind, lhs, rhs, vector_match}— the relational/vector-tree binary operator node (PromQL binary ops between two vector-shapedQueryExprs, or a vector and a scalar bridge).QueryExpr::Compare{left, op: CompareOp, right}andQueryExpr::Arith{op: ArithOp, left, right}— the scalar-expression sub-language's own binary comparison/arithmetic nodes (used insideFilter.pred,Aggregate.having,Casebranches, etc.).BinaryOpKindliterally reusesArithOp/CompareOpinternally (BinaryOpKind::Arith(ArithOp)/BinaryOpKind::Compare(CompareOp), see the doc comment onBinaryOpKindinquery_expr.rs) — so the same operator vocabulary is expressed through two different node shapes depending on which side of the relational/scalar split the expression sits on.Same discriminant reliance as Instance 1:
canonicalize.rs:83givesBinaryOp{lhs,rhs}its own recursion arm (vec![lhs.as_mut(), rhs.as_mut()]) whileCompare/Arith(canonicalize.rs:86,95) fall into the generic scalar-leaf bucket alongsideLiteral;resolve.rs:320,335givesBinaryOpits own real match arm whileCompare/Arith(resolve.rs:349,358) sit in that module's catch-all too. Three separate functions currently tell "relational, has real children to recurse into" from "nested scalar leaf" purely by which of these two variants was used.Why this is worth tracking
Both instances follow the same shape: a relational-tree node and a scalar-sub-language node exist in parallel for what is, value/operator-wise, the same thing — an artifact of #205 unifying the type (
Expr<C>folded intoQueryExpr<C>) without unifying the shapes that had accumulated on both sides of the original relational/scalar split. Filing this purely to record the observation from the #183/#184 audit; no proposed resolution here.