Skip to content

fix(query): reject STR equality keys in asof-join and window-join - #400

Merged
singaraiona merged 2 commits into
RayforceDB:devfrom
belowzeroff:fix/asof-join-str-key-nyi
Aug 14, 2026
Merged

fix(query): reject STR equality keys in asof-join and window-join#400
singaraiona merged 2 commits into
RayforceDB:devfrom
belowzeroff:fix/asof-join-str-key-nyi

Conversation

@belowzeroff

Copy link
Copy Markdown
Contributor

Problem

asof-join, window-join, and window-join1 accept string (STR) equality-key columns and then return silently wrong results — no error, corrupt data.

Root cause: all three read equality-key cells through read_col_i64, which has no RAY_STR case, so a STR column falls into the byte-wide default (((uint8_t*)data)[row]).

  • asof-join (asof_eq_lread) mismatches outright — a key that should match nulls out:

    ;; identical data, only the key TYPE differs
    (asof-join [Sym Time] tr qs)  with SYM Sym -> Bid [99 99]   correct
    (asof-join [Sym Time] tr qs)  with STR Sym -> Bid [99 0Nl]  WRONG
    
  • window-join / window-join1 read one byte per row when they sort and probe the right side. Small key sets coincide by luck, but once there are more distinct keys than fit in a byte the collisions cross-contaminate groups:

    ;; 300 distinct keys, each with its own quote; per-key minBid must equal the key
    (sum (- minBid (til 300)))  with SYM keys ->      0   correct
    (sum (- minBid (til 300)))  with STR keys -> -41605   WRONG
    

Fix

The base inner-join / left-join / anti-join use a separate STR-aware kernel (ray_join / ray_antijoin) and stay correct even with 300 distinct STR keys (verified), so this is scoped to the two window/asof kernels only.

Decline a STR equality key on either side with nyi instead of returning corrupt data (parted columns unwrapped to their base type first, matching the base join). One guard in ray_asof_join_core covers both asof call paths (plain + parted/day); one in window_join_impl covers both window-join and window-join1.

nyi (rather than adding STR support to these kernels) is the conservative bug fix: it turns silent corruption into an explicit, catchable error. Real STR eq-key support in the window/asof kernels can be a follow-up.

Tests

test/rfl/join/str_key_nyi.rfl:

  • STR eq-key -> nyi for asof-join (both operand orders), window-join, window-join1;
  • SYM eq-keys keep working and stay order-independent on the right side;
  • base inner-join / anti-join with STR keys must not be over-rejected (regression guard).

make test: 3673 of 3674 passed (1 skipped, 0 failed).

Both kernels read equality-key cells through read_col_i64, which has no
RAY_STR case: a STR column falls into the byte-wide default. asof-join
(via asof_eq_lread) mismatches outright — a key that should match nulls
out; window-join / window-join1 read one byte per row when they sort and
probe the right side, so once there are more distinct keys than fit in a
byte the collisions cross-contaminate groups and the aggregates are
wrong. Neither surfaces an error, so callers get silently corrupt data.

The base inner/left/anti joins use a separate STR-aware kernel and stay
correct, so scope the fix to the two window/asof kernels: decline a STR
equality key on either side with nyi (parted columns unwrapped to their
base type first). One guard in ray_asof_join_core covers both asof call
paths; one in window_join_impl covers window-join and window-join1.

Adds test/rfl/join/str_key_nyi.rfl covering the nyi rejection, the SYM
keys that must keep working (order-independent), and the base joins that
must NOT be over-rejected on STR keys.
@singaraiona

Copy link
Copy Markdown
Collaborator

Blocking: the new STR guard does not cover the supported 4-argument compatibility form of window-join.

The guard is inside the n >= 5 Rayforce-convention branch of window_join_impl, so (window-join left right [eq-keys] time) bypasses it and falls through to ray_asof_join / exec_window_join. I reproduced this on PR head da97945 with the same STR fixture:

(at (window-join trades quotes [Sym] 'Time) 'Bid)

It still returns [99 0Nl] rather than nyi, so silent corruption remains. The existing lang/eval/window_join test confirms this call shape is supported.

Please add the validation to the legacy fallthrough as well (or, preferably, reject STR after equality columns are resolved in the shared as-of executor), and add regressions for the 4-argument window-join and window-join1 paths. The new test currently exercises only the 5-argument window form, which is why the full suite still passes.

For context, the previous related fixes covered adaptive-width/cross-domain SYM; they did not add STR descriptor handling to read_col_i64.

@singaraiona
singaraiona merged commit 31d8970 into RayforceDB:dev Aug 14, 2026
9 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.

2 participants