fix(engine): bucket_script resolves TSVB filter_ratio's _count + null/ternary script - #95

…the bucket_script evaluator ahead of #95

Two changes with different urgency. Stating that plainly because an earlier
draft of this message claimed a process kill that shipped XERJ never had.

LIVE DEFECT — `/_msearch` skipped the script guard.

`/_msearch` built its sub-requests straight off `parse_request`, skipping
`build_search_request` and therefore the request-time script validation the
single-search path applies. That is a general bypass of the guard, not
something specific to any one script shape: a 240 KB `_msearch` body whose
items carry scripts that `_search` rejects with a 400 came back 200 on every
item. `_search/template` and `_msearch/template` skipped the same guard on
their rendered bodies. All three now run it, `_msearch` per sub-search item,
failing only the offending item with a 400 in ES's per-item error shape and
letting the rest of the batch run.

PRE-EMPTIVE BOUND — the evaluator is not recursive today, and is about to be.

On this tree `eval_script_expr` is `tokenize_script` -> `shunting_yard` ->
`evaluate_rpn`, all three iterative; `?` and `:` are not even tokens. An
80,001-byte nested-ternary source returns `None` in 4.5 us on an optimized
build. There is no stack growth to exhaust and no released version was
process-killable through this path.

The recursion arrives with PR #95, which adds `eval_tokens` and
`find_ternary_split` and re-enters the evaluator once per `?`. With #95
applied and this guard bypassed, the same 80,001-byte source overflows the
worker stack and, because the release profile sets `panic = "abort"`, aborts
the process rather than returning an error. So the bound lands first and #95
lands onto it.

`check_expr_limits` now runs on ENTRY to `eval_script_expr`, before a single
token is produced, rejecting a source past 64 KiB, past `MAX_EXPR_DEPTH`
(64) paren nesting, or carrying more than `MAX_EXPR_DEPTH` ternaries, which
matches `sql::MAX_SQL_DEPTH` and `xerj-query`'s `MAX_QUERY_DEPTH`. Ternaries
are counted rather than depth-tracked because a right-associative chain
(`a?b:c?d:e`) descends one level per `?` while never opening a paren. A
violation resolves to the same null bucket value every other unevaluable
script already produces.

Closes #111

…the bucket_script evaluator ahead of #95 (#115)

Two changes with different urgency. Stating that plainly because an earlier
draft of this message claimed a process kill that shipped XERJ never had.

LIVE DEFECT — `/_msearch` skipped the script guard.

`/_msearch` built its sub-requests straight off `parse_request`, skipping
`build_search_request` and therefore the request-time script validation the
single-search path applies. That is a general bypass of the guard, not
something specific to any one script shape: a 240 KB `_msearch` body whose
items carry scripts that `_search` rejects with a 400 came back 200 on every
item. `_search/template` and `_msearch/template` skipped the same guard on
their rendered bodies. All three now run it, `_msearch` per sub-search item,
failing only the offending item with a 400 in ES's per-item error shape and
letting the rest of the batch run.

PRE-EMPTIVE BOUND — the evaluator is not recursive today, and is about to be.

On this tree `eval_script_expr` is `tokenize_script` -> `shunting_yard` ->
`evaluate_rpn`, all three iterative; `?` and `:` are not even tokens. An
80,001-byte nested-ternary source returns `None` in 4.5 us on an optimized
build. There is no stack growth to exhaust and no released version was
process-killable through this path.

The recursion arrives with PR #95, which adds `eval_tokens` and
`find_ternary_split` and re-enters the evaluator once per `?`. With #95
applied and this guard bypassed, the same 80,001-byte source overflows the
worker stack and, because the release profile sets `panic = "abort"`, aborts
the process rather than returning an error. So the bound lands first and #95
lands onto it.

`check_expr_limits` now runs on ENTRY to `eval_script_expr`, before a single
token is produced, rejecting a source past 64 KiB, past `MAX_EXPR_DEPTH`
(64) paren nesting, or carrying more than `MAX_EXPR_DEPTH` ternaries, which
matches `sql::MAX_SQL_DEPTH` and `xerj-query`'s `MAX_QUERY_DEPTH`. Ternaries
are counted rather than depth-tracked because a right-associative chain
(`a?b:c?d:e`) descends one level per `?` while never opening a paren. A
violation resolves to the same null bucket value every other unevaluable
script already produces.

Closes #111

…sed ternaries

Combines PR xerj-org#95 and PR xerj-org#105 (xerj-org#105's head already contained xerj-org#95's commits
verbatim, so this is one stack, not two rival changes) and reworks the parts an
adversarial review measured as wrong. Rebased onto main now that xerj-org#111 has landed
as xerj-org#115; the `check_expr_limits` entry bound this relies on comes from there.

`_count` BUCKETS_PATH — TWO FORMS, ONE INVARIANT.

`resolve_bucket_script` now resolves `"<sibling>>_count"` (it looked for a
sub-agg literally named `_count`, found nothing, and nulled the bucket) and the
bare `"_count"` that Timelion's `.opensearch()` datasource emits for its
per-bucket count metric. The bare form refers to the CONTAINING bucket's own
doc_count, so `run_aggs_in_bucket` stages one under a transient `doc_count` key
for the resolver to read, then strips it back out.

The invariant, stated once because getting it wrong is what blocked the previous
attempt: whatever number a bucket publishes as its `doc_count`, a bare `_count`
inside that bucket resolves to THE SAME number. Not "the weighted count" and not
"the physical count" — the bucket aggs in this file split into two families and
`_count` has to follow each of them rather than pick a side:

  sum_doc_count(docs) — honours the `_doc_count` metadata field rollup /
    downsampled indices attach: terms, date_histogram, histogram, filter,
    filters, date_range, composite.

  docs.len() — physical rows, `_doc_count` ignored: range, multi_terms,
    ip_range, ip_prefix, rare_terms, geo_distance, geotile_grid, geohash_grid,
    adjacency_matrix, time_series, variable_width_histogram, sampler,
    diversified_sampler, nested, reverse_nested, global.

So `run_aggs_in_bucket` stages the count its CALLER is about to publish, passed
in as an argument, rather than recomputing one. Recomputing is wrong with either
function, and both spellings were measured on a two-doc corpus
`[{s:a,n:1,_doc_count:5},{s:a,n:2,_doc_count:3}]` with a `{"buckets_path":
"_count"}` sub-agg:

  staging sum_doc_count  -> range   doc_count 2 vs `_count` 8
                            multi_terms 1 vs 5, and the same split for
                            adjacency_matrix, sampler, diversified_sampler,
                            global, reverse_nested, time_series, rare_terms and
                            variable_width_histogram (10 types, not the 2 the
                            report named)
  staging docs.len()     -> terms   doc_count 8 vs `_count` 2

Passing the caller's number gives agreement for all 24 bucket types that run
sub-aggs, measured one by one — that sweep is now
`bare_count_agrees_with_every_bucket_types_doc_count`, which fails on `range`
with the recomputing version restored. significant_terms, significant_text and
missing never call into sub-aggs at all, so they have no `_count` surface.

At the TOP level of an aggs tree there is no enclosing bucket, so `_count` is
the physical size of the result set — the number `hits.total.value` carries in
the same response.

A previous version of this change asserted in a shipped comment, and in its
commit message, that `range` builds its doc_count with `sum_doc_count`. It does
not: `run_range` uses `bucket_docs.len()`, as does `run_multi_terms`. Both the
comment and this message now describe the code that is actually here.

THE EXPRESSION EVALUATOR.

Three changes, one of which is a deliberate behaviour delta:

* `null` literal, so TSVB's `filter_ratio` guard
  (`params.x != null && ...`) evaluates instead of nulling the bucket.

* Ternaries INSIDE PARENTHESES now evaluate. `find_ternary_split` only ever
  looked at paren-depth 0, and any `?` that survived into `shunting_yard` made
  it bail, so `(1?1:0)` — and therefore every nested ternary, and
  `(params.a > 0 ? 1 : 0) + 1` — silently produced a null bucket value for a
  legal Elasticsearch script. Measured as `None`, verbatim, for `(1?1:0)`,
  `(1?1:0) + 1`, `1 + (1?1:0)`, `0?1:(0?2:3)`, `(0?1:2) * 10`,
  `(1>0 ? 1 : 0) + 1` and `1?(1?5:6):0`.

  Fixed rather than rejected: rejecting would need an error channel
  `resolve_pipeline_agg_full` does not have (it returns a Value, and every
  unevaluable script degrades to `{"value": null}`), and a hard error is the
  wrong answer for a script ES accepts. `fold_paren_ternaries` collapses each
  parenthesised group containing a `?` to the number it evaluates to before
  shunting-yard sees it. Groups without a `?` are untouched.

  That adds a second recursion path, so the entry bound's argument changed with
  it: a frame consumes either the `?` it split on or one paren level, and
  `check_expr_limits` caps both at MAX_EXPR_DEPTH, giving 2*MAX_EXPR_DEPTH + 1
  frames rather than MAX_EXPR_DEPTH + 1. The doc comment says so now, and
  `parenthesised_ternaries_stay_inside_the_entry_bound` exercises a chain that
  alternates the two forms.

* `!=` ANSWERS DIFFERENTLY AGAINST NaN. This is the delta an earlier summary
  denied. `==` / `!=` are derived from one equality test so they can also
  compare the `null` literal; `!=` is now the negation of `==` instead of its
  own `(a - b).abs() >= 1e-9`. Every comparison against NaN is false, so the old
  form answered `NaN != 0` with 0.0 — "they are equal". Measured end to end on
  `params.a * params.a - params.a * params.a != 0` over one doc with
  `n = 1e308` (the product overflows to +inf; inf - inf is NaN): 0.0 with the
  old arms restored on this tree, 1.0 with the current form. `==` is unchanged,
  0.0 both ways, and non-NaN operands keep the identical 1e-9 epsilon.

  1.0 is what IEEE-754, and therefore Java, and therefore Painless say, so the
  new answer is kept — but it is a change, it is pinned by
  `ne_against_a_nan_operand_is_true_not_false` and
  `nan_operand_ne_reports_inequality_end_to_end`, and it is no longer described
  as "identical NaN behaviour".

TEST CORRECTION.

`sibling_underscore_count_matches_a_doc_count_weighted_filter_bucket` was
documented as pinning the weighted staging. It does not: with the staging
reverted to `docs.len()` it still passes, and only
`bare_count_buckets_path_matches_a_doc_count_weighted_bucket` fails. What it
actually pins is `"<sibling>>_count"` resolving at all. Its comment now says
that, and records the measurement.

…sed ternaries (#105)

Contains PR #95 verbatim, so this closes both.

`bucket_script` could not resolve `_count` as a bare-string buckets_path, which
is what Kibana's TSVB `filter_ratio` emits, and its expression evaluator had no
null literal and no ternary. Together those made a whole class of ordinary TSVB
panels return null.

`_count` NOW AGREES WITH `_doc_count`. A `_doc_count`-weighted bucket and the
`bucket_script` inside it used to disagree about how many documents the bucket
held, because the bare `_count` reported the physical document count. Verified
per aggregation type against a weighted fixture: terms, range, multi_terms,
histogram, date_histogram, auto_date_histogram, filter, filters, ip_range,
geo_distance, nested and the sampler family all report a `_count` equal to
their own `doc_count`.

PARENTHESISED TERNARIES EVALUATE. `find_ternary_split` only looked at paren
depth 0, so `(1?1:0)` — and by extension any nested ternary, and
`(params.a > 0 ? 1 : 0) + 1` — silently resolved to a null bucket value.

TWO EQUAL INFINITIES COMPARE EQUAL. Deriving `==` from `(x - y).abs() < 1e-9`
alone is wrong for infinities: `inf - inf` is NaN, NaN fails the epsilon test,
and two identical values therefore compared unequal. Measured with one document
at `n = 1e308`, `params.a * params.a != params.a * params.a` answered 1.0 where
IEEE-754 and the JLS both say `inf == inf`. Exact equality is checked before
the epsilon.

BEHAVIOUR CHANGES, each pinned by a test rather than asserted in prose:

- `!=` against a NaN operand: 0.0 -> 1.0. The old form reported `NaN != 0` as
  "equal", because every comparison against NaN is false. 1.0 is the
  IEEE-correct answer and the change is deliberate.
- A sub-agg literally named `_count`: the bucket's doc count now wins, per ES's
  reserved-path rule.
- Object-form buckets_paths at a plain numeric sub-metric now resolve where
  they previously returned null.

The expression evaluator's depth bound from #111 is underneath this, so the
recursion `eval_tokens` introduces is bounded before it can be reached.