You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While auditing which QueryExpr variants are actually exercised (see #181, #182), several node names turned out to be non-obvious about what they are or which language they're for, once you're not the person who just wrote them:
PromQL-only nodes with generic-sounding names — nothing in the name signals these only ever come from PromQL today (docs/pre-asap-ir.md's ## PromQL-specific nodes section groups them, but the type names themselves don't):
Scalar — a bare numeric literal / folded constant. Reads as a general-purpose scalar type, not "PromQL number literal."
EvalTime — PromQL time(). Name alone doesn't say PromQL, and invites confusion with SQL's NOW()/CURRENT_TIMESTAMP — verified those do not lower to this node (they fall through to a generic opaque L2Expr::FunctionCall, no dedicated node at all).
VectorFromScalar / ScalarFromVector — PromQL vector(s) / scalar(v). Accurate but generic; nothing marks them PromQL-only.
Relabel — PromQL label_replace/label_join. Reasonable name, but "relabel" is Prometheus jargon that doesn't self-explain to a SQL-background reader.
InfoJoin — PromQL info(v, [selector]). The Join suffix invites confusion with the actual Join node (SQL JOIN) even though the two share no code path or semantics.
Sample — PromQL limitk/limit_ratio series-sampling. Easy to misread as "a Scan predicate" or literally "a data sample," not "keep a subset of whole series."
Requested additions to the same cleanup, not PromQL-specific but flagged for the same reason (ambiguous/misleading name):
Merge — n-ary exact UNION ALL. Name collides conceptually with SQL JOIN/merge-join physical strategies and doesn't hint "concatenate branches, no dedup." Doc comment already has to clarify "not SetOp" every time it's referenced.
Distinct — row-level dedup (SELECT DISTINCT). Name collides with AggIntent::Cardinality (COUNT(DISTINCT col)) in conversation/code-review even though they're unrelated in shape (rows vs. a single number) — worth a name that doesn't invite that mix-up.
Summary
While auditing which
QueryExprvariants are actually exercised (see #181, #182), several node names turned out to be non-obvious about what they are or which language they're for, once you're not the person who just wrote them:PromQL-only nodes with generic-sounding names — nothing in the name signals these only ever come from PromQL today (
docs/pre-asap-ir.md's## PromQL-specific nodessection groups them, but the type names themselves don't):Scalar— a bare numeric literal / folded constant. Reads as a general-purpose scalar type, not "PromQL number literal."EvalTime— PromQLtime(). Name alone doesn't say PromQL, and invites confusion with SQL'sNOW()/CURRENT_TIMESTAMP— verified those do not lower to this node (they fall through to a generic opaqueL2Expr::FunctionCall, no dedicated node at all).VectorFromScalar/ScalarFromVector— PromQLvector(s)/scalar(v). Accurate but generic; nothing marks them PromQL-only.Relabel— PromQLlabel_replace/label_join. Reasonable name, but "relabel" is Prometheus jargon that doesn't self-explain to a SQL-background reader.InfoJoin— PromQLinfo(v, [selector]). TheJoinsuffix invites confusion with the actualJoinnode (SQLJOIN) even though the two share no code path or semantics.Sample— PromQLlimitk/limit_ratioseries-sampling. Easy to misread as "a Scan predicate" or literally "a data sample," not "keep a subset of whole series."Requested additions to the same cleanup, not PromQL-specific but flagged for the same reason (ambiguous/misleading name):
Merge— n-ary exactUNION ALL. Name collides conceptually with SQLJOIN/merge-join physical strategies and doesn't hint "concatenate branches, no dedup." Doc comment already has to clarify "notSetOp" every time it's referenced.Distinct— row-level dedup (SELECT DISTINCT). Name collides withAggIntent::Cardinality(COUNT(DISTINCT col)) in conversation/code-review even though they're unrelated in shape (rows vs. a single number) — worth a name that doesn't invite that mix-up.WindowFunc— SQL analyticOVER (...)functions. One character away fromWindow(the reserved, unimplemented streaming node — Remove QueryExpr::Window (ψ) — no producer exists, PromQL ranges always collapse to TimeRange #182), which is exactly the kind of near-miss that causes real confusion in review/grep.EvalTime— listed above; also flagged directly by the user for this pass.