Skip to content

refactor(r13): method calls onto the registry; D3 case mode, P56, P57 (R13 slice 5) - #93

Merged
TimelordUK merged 6 commits into
mainfrom
refactor/r13-slice5-method-calls
Sep 26, 2026
Merged

TimelordUK merged 6 commits into
mainfrom
refactor/r13-slice5-method-calls

Conversation

@TimelordUK

Copy link
Copy Markdown
Owner

R13 slice 5: WHERE's hand-rolled method calls move onto the function registry. Every predicate operator and method call in WHERE now evaluates through the value evaluator — one implementation per method, in every clause.

Behaviour changes

  • D3 (decided): Contains / StartsWith / EndsWith / IndexOf (method and function forms) and INSTR follow --case-insensitive like = and LIKE — case-sensitive by default. WHERE's .Contains() used to ignore case always; SELECT's never did, even under the switch. Queries relying on WHERE's old behaviour change answer; --case-insensitive restores it.
  • P56 fixed: a method over a NULL receiver is NULL, not FALSE — WHERE NOT label.contains('e') no longer selects NULL labels (both evaluators).
  • P57 fixed: any method may sit on the left of a WHERE comparison (WHERE s.ToUpper() = 'ABC' used to fail the query; only Length / IndexOf / Trim* were accepted).
  • D2 extended: a numeric receiver is searched/trimmed as its displayed text (SELECT used to error on n.Contains('5'), n.Trim()).

How

  • SqlFunction::evaluate_with_case (default ignores the mode) carries the switch into a function; MethodFunction::evaluate_method takes it; the value evaluator's three registry call sites pass it.
  • One search_text reader in string_methods.rs for the search functions, LENGTH and TRIM*.
  • WHERE delegates unconditionally; ~700 lines of its method arms, column resolver, literal readers, compare_trilean and did-you-mean suggester deleted.

Commits (pin → fix, R10 pattern)

  1. Pin method calls — 3 matrices, DuckDB-derived; file P56, D3
  2. Pin function forms
  3. Registry search functions fixed (27 entries FIXED; 179 → 180 AGREE)
  4. Pin methods WHERE rejected; file P57
  5. WHERE delegates; arms deleted (28 entries FIXED; 180 → 183 AGREE)
  6. Orphaned doc comment

Verification

  • Parity: 179 → 183 AGREE / 213; each step moved exactly the predicted cases, contract holds.
  • Rust integration 504 ✅, lib 834 ✅, Python 570 ✅, all 35 FORMAL examples ✅. Only failure is the expander_rewriters smoke example, which fails identically on main (unrelated GROUP BY error).
  • A numeric Trim() receiver was missed by the pin and caught by the presidents FORMAL example; fixed and pinned.
  • Three tests asserted the old always-case-insensitive rule; rewritten to assert D3 in both modes. No captured example output changed.
  • Perf: Contains, Length() >, NOT StartsWith over trades_100000.csv all at load time (~2.5 s), level with LIKE.

Left for later

  • Under D3: REPLACE, FREQUENCY, SUBSTRING_BEFORE/AFTER, SPLIT_PART, LEFT/RIGHT-with-delimiter don't follow the switch yet.
  • LENGTH / INDEXOF / INSTR count bytes, not characters (unpinned).
  • Next R13 step: slice 4 tail (AND / OR / NOT / CASE delegate), then slice 6.

Details: docs/ENGINE_REFACTORING.md § R13 slice 5; P56, P57, D2, D3 in docs/SQL_PARITY.md.

🤖 Generated with Claude Code

TimelordUK and others added 6 commits September 26, 2026 11:30
…P56, D3 (R13 slice 5)

Three matrices through both evaluators (default, --case-insensitive, numeric
receiver), expectations from DuckDB's contains/starts_with/ends_with/length/
instr/trim family. 33 divergences recorded: P56 (NULL receiver answers FALSE
in both evaluators), D3 (WHERE's search methods always ignore case; the
registry ignores the switch), and the registry rejecting a numeric receiver
(D2 extended to methods). Three DIFFER corpus cases via DuckDB's dot-call
syntax; parity contract holds at 212.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ce 5)

CONTAINS / INSTR / ENDSWITH as functions share the methods' implementation,
so D3 (follow --case-insensitive), P56 (NULL gives NULL) and D2 (a number is
its displayed text) apply to them too. 14 entries recorded; WHERE already
delegates function calls, so both columns agree.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nd follow --case-insensitive (R13 slice 5)

CONTAINS / STARTSWITH / ENDSWITH / INDEXOF / INSTR (and their method forms)
read both operands through one `search_text`: NULL gives NULL (P56), any
other value is its displayed text (D2), lowercased under --case-insensitive
(D3). The mode reaches a function through a new default-implemented
`SqlFunction::evaluate_with_case`, and `MethodFunction::evaluate_method`
takes it too; the value evaluator's three registry call sites pass it in.
LENGTH reads a number as its text as well.

Exactly the 27 value-evaluator and function-form matrix entries went FIXED;
WHERE's own method arms did not move. Parity moved by exactly one case,
select_method_null_receiver (179 -> 180 AGREE / 212). FORMAL examples pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e P57 (R13 slice 5)

`WHERE s.ToUpper() = 'ABC'` fails the query: WHERE's left-side method arm
knows only Length / IndexOf / Trim*. Two matrix rows, a qualified-receiver row,
and a GAP corpus case via DuckDB's `label.upper()`. Parity contract holds (213).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ders go (R13 slice 5)

A method call as a predicate, and a comparison whatever its left operand, now
evaluate through the value evaluator and the registry's method functions.
evaluate_binary_op's legacy tail and everything reachable only from it are
deleted: the Contains/StartsWith/EndsWith and Length/IndexOf/Trim* arms,
WHERE's own column resolver, literal readers, compare_trilean and
did-you-mean suggester (~700 lines).

The remaining 28 matrix entries went FIXED. Parity moved by exactly the three
WHERE cases (P56, P57, D3): 180 -> 183 AGREE / 213.

Not caught by the pin: a numeric Trim() receiver. WHERE trimmed its text; the
registry's TRIM errored, failing the presidents FORMAL example. TRIM /
TRIMSTART / TRIMEND now read a number as its displayed text (D2), pinned.

Three tests asserted the old always-case-insensitive rule; rewritten to assert
D3 in both modes. No captured example output changed. Contains / Length /
StartsWith over trades_100000.csv run at load time, level with LIKE.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@TimelordUK
TimelordUK merged commit 50cd6ba into main Sep 26, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant