Repository navigation
A column compared with a function of itself no longer collapses onto one Painless parameter (#370) - #374
Merged
Conversation
…pses onto one Painless parameter
`PainlessParam` identity was the doc-value STRING while a folding function's
method was attached to the parameter OBJECT, so the second use of a column in
one script resolved to the first object -- which by then carried the first
chain. `WHERE DATE_TRUNC(d, MONTH) < d` compared the truncation with ITSELF
(always false, HTTP 200); `DATE_ADD(d, INTERVAL 1 DAY) = d` was always true;
`YEAR(d) > MONTH(d)` folded both extractions onto one parameter and failed the
shard with `int.get(ChronoField)`. Measured on ES 8.18 before the fix.
Identity is now the column plus the FOLDED LEADING RUN of its chain
(`Identifier.contextKey`), decided by the same `startsWith(".")` test
`toPainless` folds on (`TransformFunction.foldsOntoOperand`, context-free,
keyed on renderings so two spellings of one fold share). `equals` is untouched:
a case-class identifier inherits it, and the AST must not move. Seven rules
travel together, each forced by a measurement:
1. one parameter per (column, folded run); `DATE_TRUNC ... QUARTER` binds
rather than folds and overrides the test at the site that knows
2. the UTC normalisation of a raw doc-value is PREPENDED -- a `.toLocalDate()`
narrowing is appended at coercion time, and once two uses are two objects
the order is what it is
3. `DateTrunc` reads its receiver off the REGISTERED parameter
(`rendersLocalDate`), not the column's type: `truncatedTo` is not a
`LocalDate` method
4. the interval family folds onto ITS operand's parameter (`ctx.find(base)`),
not onto "the last parameter created" (`ctx.last`): `CASE WHEN
DATE_ADD(d, INTERVAL 1 DAY) > ts THEN DATE_ADD(d, INTERVAL 1 DAY) ...`
added the day to `ts` and returned it, silently
5. a no-schema CAST declares the fold its `toPainless` really does, so
`CAST(x AS DATE) - INTERVAL 3 DAY` keys apart from a bare `x`: `THEN x`
returned `x - 3 days` -- and the bridge fixture had PINNED that value
6. a CAST is the identity on a receiver already narrowed to `LocalDate`: rule
4 keeps a coercion the old fold discarded, which is the only reason
`CAST(d AS DATE) - INTERVAL 3 DAY = <date literal>` worked before, and the
plain `CAST(d AS DATE) = <date literal>` failed the shard on main itself
7. the same for `LocalTime`
Every emission that moved was executed on real Elasticsearch: the four defect
shapes, the CASE/COALESCE interval shapes, the CAST-vs-literal shapes, and the
two bridge CASE pins (old vs new over a fixture covering every branch). Guards:
`ParameterIdentitySpec` (19, each rule seen red by mutation), the survival
sweep re-referenced to render the operand FOR its predicate, executed rows in
`PredicateFunctionResultSpec` (the #370 row flipped to truth, `YEAR(d) >
MONTH(d)`, `DATE_ADD(d, INTERVAL 1 DAY) = d`, CAST DATE/TIME) green on the five
clients / ES 6.8, 7.17, 8.18, 9.0. Parse cost measured x5 interleaved against
a control clone: ranges overlap, no signal.
Reviewed independently (three passes) before commit. Pre-existing, surfaced,
deliberately NOT fixed here and owed to one follow-up PR: a mixed
LocalDate/ZonedDateTime comparison (`CAST(d AS DATE) - INTERVAL 3 DAY > d`,
now loud where it was silently false); `LocalTime` has no `isEqual`; the
CASE-expression WHEN has no null guard on its right operand; column-vs-column
temporal comparison translated as a `range` with the other column's NAME as
date math; arithmetic over date functions drops the functions (query and
ingest); Processor/Transform contexts still collapse (`LiteralParam` keys); the
arg-coercion path re-narrows an already narrowed parameter.
Closes #370
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
PainlessParamidentity was the doc-value string (Identifier.param=doc['d'].value), while a folding function's method is attached to the parameter object (toPainless→ctx.find(base).addPainlessMethod). The second use of a column in one script resolved, throughPainlessContext.get, to the first object — which by then carried the first chain. Measured on ES 8.18 before the fix:WHERE DATE_TRUNC(d, MONTH) < dleft1.isBefore(param1), truncation folded INTOparam1WHERE DATE_ADD(d, INTERVAL 1 DAY) = dleft1.isEqual(param1)WHERE YEAR(d) > MONTH(d).get(YEAR).get(MONTH_OF_YEAR)int.get→ shard failureCASE … WHEN CAST(lastUpdated AS date) - INTERVAL 3 DAY THEN lastUpdated …THENreturnedlastUpdated − 3 daysThe
WHEREtranslator emits eachANDbranch as its own script filter, soYEAR(d) = 2025 AND MONTH(d) = 1was never wrong live; the collapse needs one expression, or one script (CASE, COALESCE,Criteria.painless).The fix — seven rules, each forced by a measurement
Identifier.contextKey), keyed on the folded functions' renderings so two spellings of one fold share (YEAR(d)≡EXTRACT(YEAR FROM d)).TransformFunction.foldsOntoOperandis the samestartsWith(".")testtoPainlessfolds on, taken context-free; the run stops at the first non-folding function (YEAR(DATE_PARSE(name, …))keys on the rawname, which theDATE_PARSEargument shares).DATE_TRUNC … QUARTERbinds instead of folding and overrides the test at the site that knows.equals/hashCodeuntouched —GenericIdentifierinherits them and the AST must not move.prependPainlessMethod): a.toLocalDate()narrowing is appended at coercion time; on one shared object the fold masked the order, on two objects…toLocalDate().toInstant()fails.DateTruncreads its receiver (rendersLocalDate) off the registered parameter — two instances of one identifier can share a parameter, and readingexprappended both renderings. Context-free the receiver is the type only, else the identity key depends on predicate order.ctx.find(base)), not onto "the last parameter created" (ctx.last). Found by review:CASE WHEN DATE_ADD(d, INTERVAL 1 DAY) > ts THEN DATE_ADD(d, INTERVAL 1 DAY) ELSE d ENDadded the day totsand returned it; with THEN/ELSE swapped it folded onto the boolean condition.toPainlessreally does, soCAST(x AS DATE) - INTERVAL 3 DAYkeys apart from a barex.LocalDate. Rule 4 keeps a coercionctx.lastused to discard — the only reasonCAST(d AS DATE) - INTERVAL 3 DAY = <date literal>worked before; and plainCAST(d AS DATE) = <date literal>failed the shard onmainitself. One rule fixes both.LocalTime(CAST(ts AS TIME) + INTERVAL 1 HOUR > …).Rules 6 and 7 repair two regressions rule 4 introduced; both were found by the independent reviewer executing ~130 renderings against a control clone, not by any suite.
Verification
Every emission that moved was executed on real Elasticsearch, before and after: the four defect shapes; the CASE/COALESCE interval shapes over a fixture covering every branch (before:
d → 2024-06-01(ELSE), after:d → 2025-01-03;e → lastUpdated=2025-01-04, where the old pin answered2025-01-01); the CAST-vs-literal shapes ([b, c]); theQUARTER,CAST,YEAR(DATE_PARSE(…)),LAST_DAY,UPPERmust-not-move shapes (byte-identical).ParameterIdentitySpec(19): the mechanism (distinct parameters iff folded renderings differ;DATE_FORMAT's outer/inner share), the defect pins, the must-not-move pins, one pin per rule — each rule seen red by mutation.PredicateTransformSurvivalSpec's 4,209-predicate sweep now renders the operand for its predicate (it rendered it in isolation, which rule 3 made a wrong reference), and itsDATE_TRUNC = CAST(…)pin — which had certified an emission ES rejects (LocalDate.toInstant) — pins the executed one.PredicateFunctionResultSpecon the five clients / ES 6.8 (rest, jest), 7.17, 8.18, 9.0: the A column compared with a function of itself collapses onto one Painless parameter — silent wrong answer #370 row flipped from the recorded-wrongSet()to the truth;YEAR(d) > MONTH(d),DATE_ADD(d, INTERVAL 1 DAY) = d, CAST DATE/TIME rows added; AND forms andQUARTERkept as controls.caseWhenExprhad pinned the wrong branch.sql1405 ·core1129 · five bridge copies 208 each · 2.12.20 cross-compile · CI lint line — green on the committed bytes.650345fa: control median 3404 µs (3200–3467), branch 3408 µs (3247–3646), ranges overlap — no signal. Measured at rules 1–3; the later rules are emission-time only.Independent review, three passes on its own clone, before commit: the first pass found rule 4's population (
ctx.last) and the pinned wrong CASE branch; the second found the two regressions rule 4 introduced; the third confirmed the repairs. The reviewer's own "order A executes correctly" claim did not reproduce — both predicate orders of that shape are loud (residual 1 below) and the pin says so.Release notes
DATE_TRUNC/DATE_ADD/DATE_SUB/extractor chains compared with, or combined with another chain of, their own column now answer correctly;CASE/COALESCEbranches over the same column with different intervals return the right value. Queries that silently returned the wrong rows will change.CAST(<date column> AS DATE) = <date literal>(and>,BETWEEN,= CURRENT_DATE) now works; it failed the shard before.LocalDate/ZonedDateTimecomparisons that were silently false now fail loudly (residual 1).PainlessParamgainscontextKey,prependPainlessMethod,rendersLocalDate/rendersLocalTime;TransformFunctiongainsfoldsOntoOperand(additive, sealed/trait-default).Residuals — pre-existing, deliberately out of scope, all in one follow-up
See #373 — nine items, each executed on real Elasticsearch before and after. Per the lead's decision they ship as one PR.
Closes #370
🤖 Generated with Claude Code