v2.5.15 - #413
Merged
Merged
Conversation
Add immutable live-tail growth with explicit partition keys, shared FILE-domain symbol handling, atomic symbol rebinding, adversarial coverage, documentation, and a runnable rollover example.
fix(parse) Fix nonstring if not defined
In RAY_JOURNAL_SYNC mode ray_journal_write_bytes checks fsync's return on POSIX and fails the write with RAY_ERR_IO, but the Windows branch ignored FlushFileBuffers' return. A failed flush there was silently swallowed, so SYNC mode reported success while the data may not have reached disk — dropping the durability guarantee the mode exists to provide. Check FlushFileBuffers (0 = failure) and return RAY_ERR_IO, mirroring the POSIX path. Windows-only branch (not built on the Linux/macOS CI matrix), so it is verified by inspection against the adjacent fsync check; the failure path is not unit-testable, like the existing POSIX one. Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com>
ray_hnsw_build sized the copied vector block as n_nodes * dim * sizeof(float) with no overflow check. Dimensions whose product wraps size_t under-allocate the copy while the memcpy — and every later distance read (vectors + id*dim) — run past the buffer. Guard the product before any allocation, mirroring the per-layer neighbor guard in the loader, and reject overflowing dimensions. This hardens the public C API boundary; the in-tree (hnsw-build ...) path sizes vectors from an in-memory list and cannot reach the overflow, so it is defense-in-depth. Add a regression test driving an overflowing n_nodes/dim pair; with the guard removed it faults under ASan (stack-buffer-overflow at the copy). Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com> Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
…334) try_load_link_sidecar read the target table's sym name into a fixed 256-byte buffer (fread of 255 bytes). A name longer than 255 bytes was silently truncated, so ray_sym_intern interned a DIFFERENT symbol and the loaded column linked to the wrong table — silent data corruption on a save/load round-trip. The writer already emits the full, untruncated name. Read the whole sidecar into a buffer sized to the file (capped at 1 MiB to bound a corrupt/oversized file), and reject a short read (fread returning fewer bytes than the file size — an I/O error or a race-truncated sidecar) so a partial name can't be interned as a different symbol either. Add a regression test that links through a 300-byte target name and asserts the loaded link_target matches; it fails without the fix. Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com> Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
hnsw_load_impl read n_nodes and dim straight from the file header and sized the vectors allocation as n_nodes * dim * sizeof(float) with no overflow check. A crafted header could make that product wrap size_t, so ray_sys_alloc under-allocated the buffer while the following fread still read the full (large) element count and wrote past the allocation — a heap-overflow write driven by an untrusted index file. Factor the check into ray_hnsw_vec_size_valid(n_nodes, dim) and reject the header before any allocation, mirroring the per-layer neighbor guard. Add a unit test that drives the helper directly (ordinary dims, non-positive dims, an overflowing pair, and the exact size_t boundary). It is tested at the helper rather than through ray_hnsw_load because an overflow-patched header is refused earlier — the huge node-level read fails first — so a full-load test could not distinguish the guard. Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com>
Escalate F-0002, F-0003, F-0004, and F-0006 into CF-0001 through CF-0004 after the required corpus census.
Rebuild the MkDocs and marketing surfaces around the Rayforce brand, add the live market demo and cloud preview, unify responsive navigation, and eliminate reload layout shifts.
* fix(null): avoid f64 null casts to integers * fix(expr): guard f64 to i64 fallback casts * fix(null): clamp finite f64 narrow casts --------- Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com>
* fix(expr): avoid null truthiness casts in fallback binary ops
binary_range's fallback OP_AND/OP_OR kernels cast the widened `double`
operand straight to `uint8_t`:
uint8_t li = (uint8_t)LV_READ(i);
`LV_READ` widens integer operands to `double` and yields NaN for float
nulls, so this had two defects:
- Wrong answers from 8-bit truncation: `(uint8_t)256.0 == 0`, so
`256 and 1b` returned false.
- Undefined behavior: casting NaN (NULL_F64) or a widened NULL_I64
(-9.2e18) to `uint8_t` is UB per C11 6.3.1.4.
UBSan flagged the latter via expr_null/diff_i64_{and,or}_raw and
expr_null/diff_f64_andor_chokes.
Route AND/OR through two truthiness helpers that compare on the widened
double and never cast it back to an integer:
- truthy_intish(v, nullv) — false for 0 and for the operand's null
sentinel. The fallback reads raw column memory, so a null arrives as
the per-type sentinel widened to double (NULL_I16 / NULL_I32 /
NULL_I64, with DATE/TIME stored as I32) rather than the NULL_I64 the
VM kernel sees; `nullv` is derived per operand from the bound pointer
type so I16/I32 nulls read as false, not just I64.
- truthy_f64ish(v) — false for 0.0 and NaN (float null).
Non-null truthiness is unchanged and null-input positions still agree
with the VM kernel (documented "AND/OR with any null operand -> 0" and
the fix_null_comparisons post-pass), keeping fallback ≡ fused.
Add regression tests pinning fallback ≡ fused for nullable I64, I32 and
I16 AND/OR operands (expr_null/diff_i{64,32,16}_{and,or}_raw).
* fix(expr): preserve near-sentinel i64 truthiness
---------
Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com>
Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
…l semantics (#341) * wip(group): parallel binary aggregates (pearson/wavg/cov) via DA path Route binary co-moment aggregators through the dense-array (DA) group path instead of the hash scatter path. Adds sum_y/sumsq_y/sumxy co-moment slots to da_accum_t + per-row accumulation + per-worker merge; emit_agg_columns already finalises PEARSON/COV/WAVG/WSUM from the co-moments. Fixes poor multi-thread scaling of by-key binary aggregates (was ~2x, DA path scales ~9-12x like stddev). Verified vs numpy; diff comparator relaxed to 1e-9 combined abs+rel (1e-12 absolute tested bit-identical summation). Includes temporary RAY_GRPPROF phase instrumentation (to remove). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): merge binary-agg Sx as double for integer x-columns wavg/pearson accumulate Sx as double even when the x column is integer (e.g. wavg(bsize,bid), bsize=I32). The per-worker merge dispatched on the x-column type -> read the double bits as int64 -> garbage at >1 worker. Force float merge for binary aggs at all 3 sum-merge sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): merge binary-agg co-moments in parallel da_merge_fn path The parallel slot-range merge (da_merge_fn, taken when n_slots>=1024) merged sumsq but not the binary-aggregate co-moment arrays (sum_y/sumsq_y/sumxy). Multi-key pearson/cov/wavg over >=1024 dense slots produced wrong results at >1 worker. Add the DA_NEED_PAIR merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(group): remove temporary RAY_NO_V2/RAY_GRPPROF instrumentation The binary-agg DA fix lands on the default path (v2 declines CHAR-keyed binary group-bys -> legacy DA), so the debug env overrides are no longer needed. 3635/3635 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(if): parallel elementwise OP_IF fill; route trivial-branch if to eager exec_if always took the 'selected' lazy-branch path, whose scaffolding (true-count, id-list build, per-branch gather, scatter) is serial over ALL rows — every if-projection ran at single-core speed regardless of -c (100M numeric if: 2.2s at any core count). 1. exec_if_eager: one shared fixed-width elementwise fill, dispatched across the worker pool for len >= 64K (SYM sides warm their runtime-id LUT serially first — sym.c frozen-table rule, mirrors window.c). STR keeps the serial append path. 2. exec_if_selected: bail to eager when both branches are trivial (column scan / scalar const) and eager fills the type combination correctly — the lazy path only pays off when a branch is an expression worth restricting to its passing rows. Mixed numeric/string shapes stay on the selected path (its per-value string conversion). 100M rows local c24: numeric if 1883->310ms, sym if 2012->306ms. dazzle c48 canonical Q22: 2658->1333ms end-to-end. make test 3635/3635. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(filter): parallel bitmap->index build in exec_filter and sel_compact exec_filter ran two sequential 0..nrows sweeps (pass-count and match_idx build) before its parallel gather; sel_compact rebuilt match_idx from the rowsel serially. Both now use the classic 3-phase compaction: parallel per-chunk/per-seg counts, tiny serial prefix, parallel fill at disjoint offsets. Lazy/morsel-backed predicates keep the sequential sweep. 100M rows local c24: 2-col where-select 143->21.7ms (1.4x -> 6.6x scaling); if+where 1040->324ms. make test 3635/3635. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf: parallel where builtin, gather_by_idx, and chunk-task dispatch - ray_where_fn: 3-phase chunk compaction on the pool (was fully serial). - gather_by_idx: fixed-width value gathers dispatched over disjoint output ranges (null-bit propagation stays serial - shared-word bit writes would race). - exec_filter/where chunk phases now use ray_pool_dispatch_n (one task per chunk); ray_pool_dispatch morselizes total_elems by 1024, so passing chunk counts gave only ~2 tasks for 100M rows. 100M rows local c24: where 88->25ms, at-gather 80->31ms, 2-col where-select 138->20ms (6.8x). make test 3635/3635. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(review): harden parallel paths per skeptic review Blockers (DA binary-agg y-column): - eligibility now requires a plain numeric/temporal y; nullable integer/temporal y stays on the HT path (da_accum_row's pair branch has no y-side sentinel machinery - nulls would accumulate as values) - an FP y with HAS_NULLS sets da_any_nullable so nn[] is allocated and the emitter divides by the non-null PAIR count, not the group count Majors: - all new parallel gates require pool->n_workers > 0 (a -c 1 pool exists with 0 workers; ring fill + atomics + rc_sync were pure overhead, and the OP_IF eager reroute lost to the selected path serially - the Q22/Q25 c1 regression) - chunked dispatch_n call sites cap chunks at 1024 = the pool's initial ring capacity, so the ring never grows (dispatch_n clamps and silently DROPS tasks if ring growth fails -> uninitialized prefix entries -> OOB writes) - sel_compact seg fill switched to dispatch_n over seg-chunks (ray_pool_dispatch over segs gave 1 task under 8.4M rows) - gather_by_idx parallel path guarded by ray_parallel_flag == 0 (leaf utility, 35+ call sites; nested dispatch would corrupt the single-producer task ring) Nits: stray time.h include, restored v2-gate comment, RAY_PARALLEL_THRESHOLD symbol in pivot.c. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(group): pair-skip y-side nulls in the legacy HT binary-agg path Unify grouped binary-aggregate (pearson/cov/scov/wsum/wavg) null handling with the scalar reducers, the v2 engine and the DA path: a null on either side of the (x,y) pair now voids the whole pair on the legacy HT route too. - ght_compute_layout: a nullable y-side sets GHT_AF2_Y_NULLABLE and routes the layout to the null-aware accumulators. - accum_from_entry_nullable: pair-skip before nn++/sums. - Entry packing canonicalizes integer nulls so the accumulator can see them: NaN in F64-packed slots (a (double)sentinel cast previously read as a huge finite value — this also fixes nullable-int x beside an FP y), NULL_I64 in int-by-int slots. - Both HT emitters (radix + serial) divided pearson/cov/scov moments by the group row count instead of the accumulated pair count — wrong results whenever a group carried any null; now divide by nn. - The all-null-group guards wrote v=0.0 after ray_vec_set_null, so the common store overwrote the null sentinel — emit NULL_F64 instead. - DA eligibility now also rejects a y shorter than the scan (OP_CONST vector literal would read out of bounds). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JZwH1gpAeLzDX2bxc4jz6s * test(agg): cross-path null coverage for grouped binary aggregates 46 assertions for wsum/wavg/pearson_corr/cov/scov over nullable inputs on all three grouped routes — v2 (plain-scan int key), DA (expression int key), legacy HT (expression key + nullable-int y; F64-packed and int-packed entry lanes) — against independently computed pair-skip truth, for all four x/y type combinations, plus an all-pairs-null group (wsum 0.0, typed nulls for the ratio/moment aggs) on every route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JZwH1gpAeLzDX2bxc4jz6s * chore(review): shared dispatch-safety gate; single filter threshold - ops/internal.h ray_par_dispatch_ok(): workers + RAY_PARALLEL_THRESHOLD + ray_parallel_flag reentrancy check in one place; applied at exec_filter, sel_compact, exec_if_eager and the where builtin (local copy there — builtins.c cannot include ops/internal.h). - exec_filter: gate and table fallback derive from one row count (fidx_rows); note that pass_count from the parallel count phase is consumed by exec_filter_vec for vector inputs. - group.c: drop the never-read da_ctx_t.agg_pair_mask plumbing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JZwH1gpAeLzDX2bxc4jz6s * chore(par): shared dispatch predicate, ring-cap constant, parallel-path test Follow-ups from the audit's non-blocking notes: - core/pool.h ray_pool_par_dispatch_ok(pool, n, min_elems): the single home for the dispatch-safety predicate (workers + element threshold + ray_parallel_flag reentrancy). The three hand-copies in ops/internal.h, lang/eval.c and ops/builtins.c are gone; all six gates call the shared one. - RAY_POOL_INIT_TASKS in core/pool.h replaces the hardcoded 1024 at the three dispatch_n chunk caps and in ray_pool_create, with a _Static_assert tying it to RAY_POOL_MAX_TASKS — lowering the initial ring capacity can no longer silently desync from the caps that rely on it. - test/rfl/query/parallel_paths_large.rfl: 200k-row coverage of every new pool-parallel branch (where, gather-by-index, exec_filter, sel_compact, OP_IF numeric and SYM fill incl. the serial LUT warm-up) against closed-form expected values. - RAY_F32 dropped from if_type_eager_ok's whitelist (if_fill_range has no F32 case; unreachable today, kept unreachable deliberately). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JZwH1gpAeLzDX2bxc4jz6s --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
binary_range's OP_IDIV kernels for narrow integer output (I64/I32/I16/U8)
computed `(intN_t)floor(lv/rv)` guarded only by `rv != 0.0`. That guard
does not catch a NaN operand (`NaN != 0.0` is true), so a null float input
yields `lv/rv == NaN`, `floor(NaN) == NaN`, and the subsequent cast to an
integer type is undefined behavior — UBSan: "nan is outside the range of
representable values of type 'long long'" at exec/expr_binary_f64_idiv_mod.
Route the cast through the ray_cast_f64_to_{i64,i32,i16,u8}_null helpers,
which map NaN to the canonical null sentinel (NULL_I64/I32/I16, 0 for the
non-nullable U8) and saturate out-of-range finite results. The null
post-pass (propagate_nulls_binary) already overwrites these positions, so
final values are unchanged — this only removes the UB and yields the
correct sentinel in-buffer. Mirrors the already-safe F64-output IDIV arm
(ray_f64_fin) and the sibling casts fixed in "avoid f64 null casts to
integers"; depends on those helpers.
Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com>
Co-authored-by: Evgen Byelozorov <belowzeroff@gmail.com> Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
Review follow-ups to the HEAD(GROUP) take: pushdown.
C1 — parted inputs. exec_group_parted reads a positive group_limit as
"stop after group_limit partitions", which holds only if every partition
yields a group; an EMPTY partition under-fills the answer. The pushdown
now skips any table carrying PARTED/MAPCOMMON columns (table_is_parted),
so the hint never reaches that path from a grouped take.
C2 — LIST-producing aggregates (top/bot). Two independent fixes:
(i) the pushdown skips a group whose aggregates emit LIST columns
(has_list_agg, set where the planner resolves OP_TOP_N/OP_BOT_N);
(ii) exec.c's OP_HEAD/OP_TAIL trims — table AND vector forms — gained a
RAY_LIST case that copies element pointers WITH a retain
(list_slice_retain). The raw byte copy could not build a LIST at
all (col_vec_new rejects it), so the column came back NULL and the
query died with "table add_col: column must be a vector". That was
reachable WITHOUT any group-by — `select {a b} from: L take: 2`
over a table with a LIST column failed before this commit — and
aliasing cells without a refcount would double-free (issue #355,
same reason exec_filter_head gathers LIST columns separately).
I1 — take: is evaluated once again. apply_sort_take takes an optional
pre-evaluated value; the grouped path passes the value it already
computed for the pushdown decision instead of letting apply_sort_take
re-evaluate a possibly side-effecting expression.
I2 — sg_shape_eligible no longer bails on a non-zero group_limit. The
slice-group kernel computes every group and the caller's HEAD trims, so
the limit is a hint there too; bailing dropped where+by+take shapes onto
the generic ladder for no reason.
Tests: highcard_group.rfl gains the LIST-agg take shapes, the plain
HEAD/TAIL-over-LIST-column repro, a take:-evaluated-once counter, and a
parted store with an empty partition under grouped takes.
Round-2 review follow-ups. 1. The parted regression test used 3 rows per partition, below exec_group_parted's cardinality gate (est_groups*100 > rows_per_part), so it stayed on the concat fallback and never consulted group_limit — it passed even with the bug present. Regrown to 1000 rows across 5 partitions with partition 2 empty, it now reproduces: on f1aaa73 `by: part take: 2` answers [1] instead of [1 3] and take: 3 answers [1 3] instead of [1 3 4]. Fails there, passes here. 2. apply_sort_take's pre-evaluated take: parameter is now a DECODED take_pre_t {kind, a, b} stack struct instead of a retained ray_t*. The owned pointer would have leaked on the ~24 early-return error and OOM paths between the pushdown site and the final apply_sort_take call; a stack value cannot. The take: expression is still evaluated exactly once — apply_sort_take re-materializes the value locally. 3. .superpowers/ (working notes) was swept into the previous commit by `git add -A`; untracked here, kept on disk, and ignored from now on.
When every group key is a plain fixed-width integer lane (no GUID/STR indirection, no F64), the fat-entry pipeline now treats the key tuple as one packed value. Four changes, all measured on the 10M on-disk splayed ClickBench store: - ght_lanes_copy/ght_lanes_equal: key regions are a whole number of 8-byte lanes, so <=8 lanes copy/compare through straight u64 loads/stores instead of libc calls that re-dispatch on a runtime size. A dwarf call-graph profile put 14.6% of q18 in __memmove_avx, all of it these 24-32 byte moves in radix_buf_push and group_probe_entry. - Null-mask elision: the mask words only keep a NULL key distinct from a 0/"" key, so they are elided when every key column is provably null-free. ght_compute_layout takes the key vectors to decide; pivot.c passes NULL and keeps them. The two row-side readers are guarded (ght_null_words_at substitutes a shared zero word). - OP_COUNT reserves no entry value slot: it is group size and its emit reads the row count, never a staged value. The phase-1 agg packers now index by the layout's agg_val_slot instead of a running counter, so a valueless agg cannot shift a later agg's slot. Together these take q18's fat entry from 56 to 40 bytes, q16's from 48 to 32. - ght_hash_lanes: one wymum/wymix avalanche over the whole key region instead of nk hashes plus nk-1 combines (q18: 3 multiplies, not 10). Partition bits (hash>>16) and slot bits (hash&mask) stay independent. All seven key builders, including hash_keys_inline used by rehash/merge/lookup, converted together. - radix_phase1_fn stages packed null-free tuples column-major over a 256-row morsel, hoisting read_col_i64's per-key jump-table dispatch (~6% of q18 in mispredicted indirect jumps) out of the row loop. Row order, entry bytes, hash and partition are unchanged. Every other key shape (STR/GUID/F64/nullable/wide) keeps its existing loop verbatim. 10M splayed store, min-of-7, back-to-back A/B vs 2dd36e9: q16 121.6 -> 87.5 ms (-28.1%), q18 209.9 -> 173.3 ms (-17.4%), q15 159.7 -> 132.9 ms (-16.8%), q33/q34 -4%, q13 -0.6%, q17 -0.5% (no regressions). Rendered output of q13 q15 q16 q17 q18 q33 q34 at 10M is byte-identical to base (md5 match). Report: .superpowers/q18-packedkey-report.md
Review follow-ups to 5784c19. 1. radix_v2_phase1_fn's partition-major morsel path was DEAD in base: its `null_words == 0` gate was unsatisfiable because null_words was floored at 1. The null-mask elision made it reachable, and it still staged a per-key ray_hash_i64 + ray_hash_combine hash while hash_keys_inline — used by group_ht_rehash, group_ht_rebuild_slots and group_merge_row — now returns ght_hash_lanes for the same layout. After a worker HT rehashed, phase 1 probed with the stale hash, missed, and inserted a DUPLICATE group row; phase 2's merge folded the duplicates so answers stayed correct, which is exactly why a result diff could not catch it. Now hashes through ght_hash_lanes for packed layouts (and skips the dead per-key mixing). Measured with a temporary in-HT duplicate detector on a 2.5M-row 2-key count-only query over 306 worker HTs: 4,539,688 mis-probed rows before, 0 after. 2. Packed eligibility is now a whitelist of the types read_col_i64 decodes deliberately (BOOL/U8/I16/I32/I64/DATE/TIME/TIMESTAMP/SYM) instead of `!= RAY_F64`. read_col_i64's default arm reads one byte, so an un-enumerated fixed-width type (RAY_F32) would have been silently mis-loaded and hashed as the key identity. 3. pivot.c: guard the off_nn index with `s >= 0` the way group.c's emit paths do (safe today only by an off_nn coincidence), and record why pivot must keep passing key_vecs = NULL to ght_compute_layout — its ingest reads the key region's null word raw and needs null_words >= 1. 4. ght_lanes_copy/ght_lanes_equal assert their whole-lane precondition under DEBUG/RAY_HARDENED (what `make test` builds), free in release. 5. group_packed_key.rfl reworked: the 300K sizing never reached the code under test — instrumentation showed the morsel stager was never executed. Key cases are now 2.5M rows with the emit filter armed, and each was verified with temporary probes to reach a specific builder: morsel stager (nk=2 nv=0 contiguous, nk=2 contig=0 indexed gather, nk=3 nv=2), the packed row-major builder (nullable, null_words=1), and the fused v2 morsel with packed=1 (the regression case for #1) and packed=0. Also restores the F64 ±0.0 case in a build-independent form and records the finding: the group count for [0.0, -0.0, 1.0] differs between release (-fno-signed-zeros folds -0.0, 2 groups) and debug (3 groups, because group_keys_equal compares raw bits while ray_hash_f64 normalises) — pre-existing, F64 is not packed, filed separately. Fixes the false "past the 2-lane / 16-byte packing width" comment: eligibility is per-key type, with no tuple-width cap. 10M splayed store, min-of-7, back-to-back A/B vs 2dd36e9: q16 122.1 -> 88.7 ms (-27.4%), q18 209.6 -> 177.5 ms (-15.3%), q15 -14.0%, q33 -3.5%, q34 -1.6%, q13 +0.4%, q17 -0.8%. Rendered output of q13 q15 q16 q17 q18 q33 q34 at 10M remains byte-identical to base (md5 22e6fe0cd8f0eacc5c3f5b48c207b8e9). Report: .superpowers/q18-packedkey-report.md
…unique group-bys
ClickBench q32 (2 narrow keys, count+sum+avg, ~1 group per row) spent 32% of
its runtime on a single instruction: the ht->slots[slot] load in
radix_v2_phase1_fn. At 10M rows it builds 8 workers x 4096 partitions = 32768
worker hash tables (~134 MB of slot arrays); consecutive rows hit different
partitions, so nothing is cache-resident and nothing is prefetched.
The fix is ordering, not fewer instructions:
* radix_v2_phase1_fn's partition-major morsel path — which already stages a
1024-row morsel, counting-sorts it by partition and probes with an 8-entry
slot-line prefetch — was gated on need_flags == 0 (count-only). It now also
serves GHT_NEED_SUM, the only other need-flag the v2 pipeline can see
(exec_group_run admits only COUNT/SUM/AVG; group_merge_row supports exactly
{0, SUM}). Agg inputs are read at probe time from the staged source row, so
the morsel arrays and the entry bytes are unchanged. The ght_hash_lanes
lockstep branch in the staging loop is untouched.
Results are preserved even for f64 SUM: the counting sort is stable and a
group lives entirely inside one partition, so rows of a group are still
visited in source order; only the interleaving between partitions changes.
* The agg-packing block is factored into radix_v2_pack_aggs, shared by the
morsel-staged and row-major builders so they cannot drift.
* group_ht_t gains an optional one-shot grow_cap (slot-count target, 0 = plain
doubling). Near-unique worker tables climbed 256 -> 512 -> 1024, re-hashing
and re-inserting every live group on each rung (386 re-inserts against 305
real inserts on q32). radix_v2_phase1_fn seeds it from 2*v2_exp, the
existing per-(worker,partition) row budget at the 50% rehash load factor —
no new tunable. Applied at first GROWTH rather than first allocation: a
table that grows has proven it is near-unique, whereas pre-sizing every
table cost q15 (density ~0.25) 5%.
* Makefile: -falign-functions=64 in RELEASE_CFLAGS. Not an optimisation —
without it, adding these ~120 lines pushed exec_group_sp_dyn_emit (45% of
q33/q34, a path this change never executes) off a cache line and cost those
queries a reproducible 14%. That noise floor is larger than most real
optimisations. With the flag on both sides the artefact disappears.
10M splayed store, min-of-5, three interleaved CAND/BASE reps:
q32 644.6 -> 419.8 ms -34.9%
q31 102.3 -> 87.1 ms -14.9%
q15 140.2 -> 135.8 ms -3.2%
q13/q14/q16/q17/q18/q33/q34 all within 1%
Byte-identical on q13 q15 q16 q17 q18 q33 q34. Across all 43 queries only
q21/q31/q32/q39 differ, all inside all-tied `desc take 10` blocks; the
unmodified base binary produces different tie selections at different thread
counts for all four (q31 needs -t 5 to show it).
Test: group_packed_key.rfl gains a near-unique 2.5M-row count+sum+avg case
with desc/take; instrumentation confirms it executes both new paths
(need_flags=1 morsel stager; a 256->1024 grow_cap jump).
Full report: .superpowers/q32-unique-groups-report.md
Review of 16bd46d: grow_cap was applied in full on the strength of a single rehash, on the premise that a growing table has "proven it is near-unique". That premise is false — a rehash only proves the table just crossed ht_cap/2 groups, a lower bound on its cardinality. Since ht_init_cap is clamped to 256, ANY table crossing 129 groups took the whole row-derived ceiling (4096 slots + 2048 rows at 10M), making worker-HT memory O(rows) instead of O(groups). Measured on 10M-row 2-key in-memory shapes (the ClickBench single-key group-bys route away from the v2 pipeline on a splayed store, so they never showed it). Worker-HT bytes, summed over all 32768 tables: count-only, 2.56M groups base 501 MB -> v1 640 MB (+28%) -> now 512 MB count+sum+avg, 1.30M grps base 448 MB -> v1 896 MB (+100%) -> now 512 MB count+sum+avg, near-uniq base 893 MB -> v1 896 MB (+0%) -> now 891 MB Peak RSS (/usr/bin/time -f %M) on the middle shape at -c 2: 812 -> 976 -> 812. Two bounds: * group_ht_rehash skips ahead at most ONE extra doubling per rehash: max(ht_cap*2, min(grow_cap, ht_cap*4)). A table that stalls right after a jump now over-allocates its slot array by 2x and nothing else. * group_ht_grow no longer consults grow_cap at all. The row array is where nearly all the bytes are (row_stride 24-48 B vs a slot's 4 B) and growing it early buys nothing: a row growth is one realloc/memmove, whereas a slot growth re-hashes and re-inserts every live group. Plain doubling keeps it O(groups) — exactly base — which is what restores RSS. Cost, reported rather than hidden: q32 gives back 96 ms. q32 645.7 -> 515.7 ms -20.1% (was -34.9% unbounded; gate is >=15%) q31 102.2 -> 86.3 ms -15.6% q13/q14/q15/q16/q17/q18/q33/q34 all within 1% __memmove_avx is now q32's largest recoverable item (8.9%) — it is the row ladder this commit deliberately restored. Recovering it needs per-table occupancy history; not attempted here. Also: * -falign-functions=64 added to HARDENED_CFLAGS, whose own comment requires it not to diverge numerically or in flags from release. * grow_cap's comment in internal.h and both call-site comments in group.c corrected: it is a CEILING approached one doubling at a time, and the trigger proves a lower bound on cardinality, not near-uniqueness. * Report: RSS table added as gate 6, perf table updated with the trade, and the tie-order proof redone with -c (cores). The original proof used -t, which is --timeit (profiler enable) and varies nothing. With -c the base binary picks a different tied top-10 at essentially every core count for all four affected queries (q21/q31/q32/q39) — a far stronger result. Gates: make release + make hardened warning-clean; make test 3690/3690; q13 q15 q16 q17 q18 q33 q34 byte-identical to 701c7b8 at 10M.
`select ... by ... desc: <agg> take: N` kept only the globally best N groups before the phase-3 emit, but the whole block ran on one thread: a bounded heap over EVERY group of EVERY partition, a keep-bitmap sized by the total group count, a compaction pass over every group, and a slot rebuild that cleared every partition's full slot array. Instrumented at 100M on a 16-core box that block is 1.11 s of the 16.6 s ClickBench suite -- 6.7% of the whole run, single-threaded (q32 337 ms, q18 172, q16 81, q15 78, q33/q34 66). - The global top-k is a subset of the union of the per-partition top-ks, so the scan becomes a per-partition dispatch (topn_scan_fn) plus a merge over the survivors. The retained SET is unchanged: the heap replaces on strict improvement, so it keeps exactly the k best under the total order (value, push position), and push position stays (partition, gid) -- each worker sorts its survivors back into gid order and the merge walks partitions in ascending order. topn_heap_push is one shared helper so the pre-pass and the merge cannot drift. Admission is a reduction test derived from the data (staging <= 1/8 of the groups, i.e. under 2 bytes per group against the >=24-byte rows already resident), not a tunable; the serial scan remains for a single worker, a single partition, allocation failure and any shape that fails the test. - Compaction now runs off the <= k_take survivors sorted by (partition, gid) -- the same in-place moves as the bitmap walk, in O(k_take + n_parts). The bitmap is gone. - Before the slot rebuild, ht_cap is shrunk to the smallest power of two >= 2*survivors (the load factor group_ht_rehash already targets), so the rebuild clears a handful of slots instead of a gigabyte. The allocation is untouched and only shrinks; nothing inserts into these tables again (phase 3 reads rows, the holistic re-probe only probes). The rebuild itself is dispatched, and only partitions whose grp_count changed are rebuilt. 100M / -c 16: suite 16618 -> 16110 ms (-3.1%); q16 -7.7%, q34 -6.5%, q33 -6.2%, q15 -5.2%, q18 -4.8%, q32 -4.8%, q31 -4.2%; nothing regresses. 10M: q13 q15 q16 q17 q18 q33 q34 byte-identical to ead2fff (q21/q31/q32/q39 are the known run-to-run tie-nondeterministic set, in base too). Peak RSS flat on mid-cardinality and near-unique shapes. make test 3690/3690. Full phase decomposition, and the three serial phases this does NOT fix (phase-2 fusion of the scan, the single-threaded minute() extract at 24% of q18, the single-threaded agg_unpack_key_col at 27% of q13), in .superpowers/scaling-ceiling-report.md.
Review found a crash in f2fba93. `ray_pool_dispatch_n` drains its tickets WITHOUT running fn once pool->cancelled is set -- i.e. on Ctrl-C, exactly on the long queries this path optimizes. The topn_scan_fn staging block was scratch_alloc'd, so the merge then read garbage item_cnt[p] and walked off the end of items[] (reviewer reproduced a deterministic SIGSEGV by emulating a skipped task). Two fixes, both needed: - scratch_calloc the staging block, so a task that never runs leaves a well-defined 0 rather than stack garbage; - add the cancel bail after the topn_scan_fn dispatch, matching every other dispatch in this driver. Spelled out instead of CHECK_CANCEL_GOTO because this block owns scratch the cleanup label does not know about and must release it before the goto. The same check follows the topn_rebuild_fn dispatch -- a skipped rebuild leaves slots pointing at pre-compaction gids. Also from review: - topn_heap_push's docstring claimed "among equal values the earliest pushed wins", which is false (k=2 desc, push 5,5,7 evicts the FIRST 5: sift-down may promote either equal child). Replaced with the argument that actually holds -- a full per-partition heap holds the k best of its partition while the global heap holds the k best of a superset, so its root is never worse and it would reject anything the local heap rejects, leaving it unchanged; the pre-pass elides only no-op pushes. The two load-bearing conditions (per-partition cap is k_take itself, never a k/n_parts share; merge feeds candidates partition-ascending and gid-ascending) are now stated so a future edit cannot silently break them. - The k_take width comment was rewritten rather than deleted: match_group_desc_count_take does cap take_n at 1024, but the count-distinct rewrite (query.c:3860) sets top_count_take from any positive int64 with no cap, so the 64-bit arithmetic stays -- as a guard, not a live shape. Gates re-run on this build: make release warning-clean; make test 3690/3690; 43-query identity at 10M on an idle box differs only on the known tie-nondeterministic set (base-vs-base differs on the same four); 100M -c16 q18 1250.8 -> 1166.1 (-6.8%), q32 2920.3 -> 2769.1 (-5.2%), q16 -6.0%, q33 -5.6%, q34 -5.4%, q15 -4.9%, q13 -3.0%, q17/q20/q28 in noise. Report updated with the review round, with q40 identified as a FIFTH member of the tie-nondeterministic set (proven in base under CPU load; it does not even use the new parallel pre-pass), and with the pre-existing `asc: <agg> take: N` returns-largest-N bug the review surfaced.
`minute(EventTime)` on 100M rows was 299 ms with zero workers — 24% of q18 and the largest single serial phase left in the ClickBench suite. All four whole-column temporal kernels had the same wall: ray_temporal_extract / ray_temporal_truncate (the eval builtins minute/ss/hh/yyyy/date/time) and exec_extract / exec_date_trunc (the DAG twins, reached by writing (minute ts) or ts.date inside a select). Each is a pure elementwise map, so the existing kernels are lifted verbatim into (ctx, start, end) range functions and chunked over the pool via ray_pool_dispatch above RAY_PARALLEL_THRESHOLD — the same engine-wide gate expr_eval_full uses, which also enforces the two safety conditions (n_workers > 0, and not already inside an in-flight dispatch: the task ring is single-producer). No new knob; below the threshold the identical range function runs serially. The DAG pair switches to ray_morsel_init_range, and issues ray_morsel_init's one-time mmap readahead hint on the main thread so it fires once per column rather than once per task. The null hazard is not the null bitmap the phase decomposition predicted: there is no bitmap. ray_vec_set_null_checked writes a type-correct NULL_* sentinel into the payload, so per-row null writes are already disjoint and chunk alignment is irrelevant. What races is the tail of that same function — `attrs |= HAS_NULLS` plus the SORTED clear — a read-modify-write of one shared byte per null row. So the workers never touch attrs: each writes its own sentinels and raises one atomic flag, and the caller folds it into result->attrs once after the dispatch joins. That is byte-identical because the destination is always a fresh ray_vec_new (attrs == 0, no index, not a slice), where ray_vec_set_null reduces to exactly "sentinel + HAS_NULLS". A cancelled dispatch needs no extra guard here: the output vector is a private fresh allocation this op alone owns and the caller discards on cancel, and any_null is a stack atomic initialised to 0. No scratch block or shared index is left half-built. 100M, -c 16, min-of-3, interleaved: the minute op 299.5 -> 72.4 ms (w=16 par=15.9x), q18 1166.2 -> 942.2 ms (-19.2%); at -c 8, 1306.8 -> 1086.1 (-16.9%), so the c8->c16 ratio goes 1.12x -> 1.15x. q13/q15/q16/ q17/q20/q28/q32/q33/q34 within +/-0.5%; q14's +2.4% did not reproduce (three further passes: -0.6%). 10M local: q18 -19.3%, rest within noise. 4.1x rather than 16x because the kernel streams 1.6 GB in 72 ms ~ 22 GB/s — it is now at the DRAM roof, not thread-bound. 10M byte-identity vs bd9e743 over all 43 queries: q13 q15 q16 q17 q18 q33 q34 identical; only q21/q31/q32 differ, and two base-vs-base passes differ on q21/q31/q32/q39 (88 lines vs 30), so the candidate's diff set is a strict subset of base's own run-to-run tie variance. test/rfl/temporal/parallel_extract.rfl pins it: 2.6M-row columns with and without nulls, the nulls placed at index 1310720 (64-aligned and exactly 160x the 8192-row dispatch grain, an interior task seam) and 1310723 (mid-word in the next task), every row checked against a closed form over (til N), plus the DAG twins and the sub-threshold serial path. The nil? assertions are what pin the HAS_NULLS fold — without it ray_vec_is_null's fast-path gate would call every row non-null. make test: 3691 of 3691 passed. tsan at -c 8 on the nulls-at-boundaries file: zero races in temporal.c (the three ray_heap_gc/ray_heap_init reports are pre-existing and reproduce in two lines that never call a temporal kernel).
The v2 radix group-by built each result key column with a single-threaded loop over every output row (agg_unpack_key_col), un-packing the per-group packed key out of the per-partition buffers in first-seen order. At 100M that is 137-149 ms of q13 (27% of the query) on the main thread while every worker idles, and it anti-scales: more cores made it slower. Split the function into an allocate half (agg_unpack_key_col_new) and a range half (agg_key_emit_range) that fills rows [start,end) of ALL key columns, then dispatch it over disjoint output-row ranges — the same shape and the same RAY_PARALLEL_THRESHOLD gate as the phase-3 finalize dispatch right below it, so bounded-emit shapes (the HEAD(GROUP) hint, e.g. q17's 10 rows) and low-cardinality group-bys keep the identical serial call. No new tunable. Race-free with no attrs fold: write_col_i64 is a pure payload store at index i and the HAS_NULLS flag is copied wholesale from the source column at allocation time, before the dispatch — workers touch no shared header field. A cancelled dispatch can leave rows unwritten (an unwritten SYM slot is an out-of-domain id), so the dispatch is followed by a cancel bail that releases the half-built columns. The destination-handle array is a scratch carve, not a fixed [16] stack array: the <=16 key cap belongs to dense direct-index routing only, and a 17-key group-by is exactly what dense declines and radix accepts. ASan caught the stack version overflowing; the stale "admission-bounded <=16" comment that suggested it is corrected. 100M, -c 16, min-of-3 interleaved: q13 523.2 -> 404.2 ms (-22.7%); -c 8: 537.1 -> 422.0 (-21.4%). q15/q16/q17/q18/q20/q28/q33/q34 all within +-1%. 10M local: q13 -8.9%. Byte-identical output at 10M on q13/q15/q16/q17/q18/q33/q34. New test test/rfl/group/radix_key_emit_parallel.rfl: a 2.6M-group W32 SYM emit and a 17-key mixed-width emit, both checked per row against the source columns, plus the take-bounded serial shape.
The top-N emit filter's per-partition scan (f2fba93) ran as its own dispatch AFTER phase 2, re-reading every group row of every partition from DRAM — 62 ms on q18 / 129 ms on q32 at 100M, and bandwidth-bound (~36 GB/s of ~50), so more threads could not recover it. Each partition's top-k is a partition-local property, so it now runs at the END of the phase-2 task that BUILT that partition, while its rows are still in that core's cache: topn_fuse_partition(), wired into both radix_phase2_fn (fat-entry) and radix_v2_phase2_fn (direct-insert). The v2_emit block consumes the stash and skips its scan dispatch entirely; the standalone topn_scan_fn pre-pass and the serial scan stay as the paths for everything fusion cannot cover. Identity: the fused scan is arithmetically the same scan — shared topn_heap_push, per-partition cap == k_take, survivors sorted back into gid order, merge walking partitions ascending — so both load-bearing conditions of the retained-set proof hold unchanged. Ordering-agg resolution (order_off / desc_dir, and the "this shape is not servable" test) is hoisted above the parallel section: phase 2 needs it at dispatch time, and the emit block now reads the same values, so the two scans cannot drift. Admission: the stash is n_parts x k_take, sized before any group exists, so the standalone path's reduction test against the actual group count is made here against the row count that bounds it (n_parts * k_take <= radix_rows / 8, i.e. under 2 bytes per scattered row against the >= 16 bytes phase 1 already holds). No new env knob. Cancel-safety: item_cnt is calloc'd and each phase-2 task zeroes its own entry before any early continue/return, so a partition whose task was drained by a cancelled dispatch contributes 0 candidates rather than garbage; the existing CHECK_CANCEL_GOTO after the phase-2 dispatch is what guards the stash. Verified by emulating a fully skipped scan (the bd9e743 repro): no crash, and the gate queries stay byte-identical because the heap then stays empty and the compaction is skipped.
`select … by … asc: <agg> take: N` arms the same emit filter as the
`desc:` form — query.c's match_group_desc_count_take sets .desc = 0 for
it — but two consumers force-set the direction back to "largest first"
whenever the ordering agg was COUNT: the v2_emit direction hoist that
feeds topn_fuse_partition / topn_scan_fn, and group_emit_filter_trim.
The parallel radix path therefore returned the LARGEST N groups,
byte-identical to the desc answer, while `-c 1` (no pool, no radix emit
filter) returned the correct smallest N — the answer changed with core
count.
.desc is now authoritative everywhere:
- the two COUNT coercions are gone; topn_heap_push's TOPN_BETTER
predicate is direction-symmetric, so the fused per-partition heap,
the standalone scan, the merge and the compaction all follow the
request's direction with no further change;
- the two arming sites that relied on the coerced default state it
(try_count_distinct_v2_rewrite's desc-only rewrite, and the
min_count_exclusive heavy-hitter matcher);
- use_emit_filter — the keep-min count trims (da_count_emit_keep_min
and the dense/sparse emits) — is inherently largest-first and would
drop exactly the rows an asc take asks for, so a take-bounded asc
shape no longer arms it and falls through to the full group result
plus the downstream sort+take. A pure min_count_exclusive filter
(no take) is direction-agnostic and is unaffected.
test/rfl/group/topn_asc_take.rfl pins asc/desc at 2.5M rows on the
armed parallel path (COUNT 2-key and single-key, SUM, MIN, MAX; group
counts are all distinct so tie order cannot mask a flip) — it fails on
d315102. The three group_extra emit-filter unit tests build the
filter struct by hand and relied on the coerced default; they now set
.desc = 1, and the i16 one additionally pins that a .desc = 0 filter
does not drop the smallest group.
make test 3693/3693. 10M ClickBench (splayed, on-disk): q13 q15 q16
q17 q18 q33 q34 byte-identical to base, and the desc-take family shows
no slowdown (q13-q18/q33/q34 min-of-3 within noise both directions).
…#407) ray_hash_f64 normalises -0.0 -> +0.0, but group_keys_equal / ght_lanes_equal compare the raw 8 key bytes. A column carrying both bit patterns therefore hashed -0.0 into +0.0's slot chain and never compared equal to it, so -0.0 formed its own group. Debug builds show it with a literal [0.0 -0.0 1.0] (3 groups); release hid the literal case behind -fno-signed-zeros, but any runtime-produced -0.0 — a multiply, or data read from disk — still split. Every F64 group-key builder now reads its key through one shared helper, group_key_f64_bits, which folds the -0.0 bit pattern to +0.0 BEFORE both the key store and the hash, so hash and compare see identical bits on every path (the ab5053a lesson): the inline-STR fat-entry builder, group_rows_range, radix_phase1_fn, both radix_v2_phase1_fn loops, reprobe_flat_gid and exec_group_run's regroup. The packed lane path and the morsel stagers read through read_col_i64, which does not decode F64, so F64 stays off packed_key and needs no change there; the DA/sp_dyn and agg_engine key paths whitelist integer key types only. Because every contributing row now stores canonical bits, the merged group's emitted key is bitwise +0.0 regardless of which row opened the group or which worker got there first — order- and thread-independent. query.c's group first-idx probe gets the same treatment: it compares gk_vals bit-wise against the source column while hashing through ray_hash_f64, so both sides must be canonical or a -0.0 row would fail to claim its own group's first index. NaN policy: NaN is deliberately NOT canonicalised. Distinct NaN payloads hash distinctly AND compare distinctly, which is already self-consistent — the defect was an asymmetry between hash and compare, and NaN has none. Canonicalising would also fold 0Nf (NULL_F64) in with a runtime-produced NaN; SQL nullness rides on the key null mask, not on the payload, so null-key grouping is byte-identical to before and non-sentinel NaNs group by bit pattern. Also corrects two stale comments claiming F64's null bit pattern is -0.0. NULL_F64 is __builtin_nan(""); what actually collides with a zero key is the all-zero key slot a null writes, which reads back as integer 0 / +0.0. Tests: group_packed_key.rfl's build-dependent ±0.0 note becomes the fixed contract (2 groups, both builds); new group_f64_signed_zero.rfl covers runtime -0.0 single/multi-key, a WHERE-filtered group-by, 0Nf null keys, and 3M-row low-, mixed- and ~200K-cardinality F64 keys through the parallel radix pipeline; new lang/select_by_f64_signed_zero_key pins the +0.0 representative at the BIT level (ray_format normalises -0.0 on output, so no .rfl assertion can see it).
…y note The F64/F32 arms used the exact 'v == 0.0 ? 0.0' idiom that release's -fno-signed-zeros folds to a no-op (the #407 trap), making (mode f) build-dependent for +/-0.0. Also scopes the helper's NaN paragraph to columns without HAS_NULLS (with the attr, x != x makes every NaN payload a null and the mask folds them first).
…low UB (#409) take and drop turn their raw i64 count into a magnitude with `n < 0 ? -n : n`, so a count of INT64_MIN (0Nl, or the literal -9223372036854775808) hit `-INT64_MIN` — signed-integer-overflow UB (UBSan: src/ops/collection.c). It fired in every scalar-count take branch (vector, string, char, scalar, list) and in ray_drop_fn. take: reject an INT64_MIN count once, up front, before dispatching to the per-shape branches — its magnitude is unrepresentable as int64 and could never be allocated (a range error, which is what the downstream negative-capacity check already produced for the vector path, minus the UB). drop: a drop-from-end of that magnitude removes the whole collection (|n| >= len), so treat it as cut == len (empty result) — matching (drop x -N) for any N >= len. Normal positive/negative takes and drops are unchanged, and the table/dict paths recurse through these kernels. Adds regressions to collection/take.rfl (all take shapes) and collection/drop_cut_rotate_cross.rfl.
date_to_ymd computed offset = days + 730119 in int32, overflowing for days above INT32_MAX - 730119. (as 'DATE 2147483646) tripped UBSan at cal.h:50 and printed a garbage negative year. Widen the decode path (and date_years_by_days) to int64; re-encode stays int32-exact for the parse path. Pins decode+re-encode identity in test/rfl/temporal/date.rfl.
…ication by: (#406) * fix(query): support update over parted tables and prevent column duplication by: Two bugs in ray_update hit parted (and, for the second one, flat) tables: 1. MODIFYING an existing column of a parted table failed because the update path read the RAY_PARTED_BASE wrapper type / MAPCOMMON segment shape through ray_vec_new, ray_data and the per-group gather, none of which understands a parted column (error: 'expression type I64 does not match ? column', by: path: 'group: argument must be a vector'). Flatten the whole input table once, the way select does. 2. A by: update on an EXISTING target column appended the aggregate as a second column with the same name instead of replacing the original, so the schema became [k v w w] and 'at' kept reading the stale value. ray_table_add_col always appends, so skip source columns that the update dict replaces when copying the initial schema. Adds regression coverage for both in update_parted.rfl and corrects the by: broadcast expectations in query_update_coverage (the aggregate now lands on every row of its group, kdb style). * fix(query): preserve update by schema order * fix(query): scatter vector-valued update-by results instead of zeroing An `update {col: <expr> by: k}` whose per-group expression returns a vector (valid kdb, e.g. `update v: 2*v by k`) wrote nothing: the broadcast loop only handled atom results, so the freshly memset-zeroed output column was left all-zero — silent data loss that the earlier duplicate-column bug had masked. Add a scatter branch symmetric with the atom broadcast: when the result is a vector of the group's size, store element r at idxs[r]. A vector whose length is neither 1 nor the group size has no row-aligned meaning, so reject it with a loud `length` error rather than leave zeros in place. Also zero-fill the ngroups==0 output column to match the per-group path (uniform, no allocator garbage), and document in the storage guide that `update` over a partitioned table materializes the whole table in memory and does not write back to the store. --------- Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
* fix(arith): keep integer div exact and UB-free in ray_idiv_fn div routed integer operands through doubles, silently corrupting every result above 2^53 (div 9007199254740993 1 -> 9007199254740992) and tripping UBSan at q == 2^63 (div -9223372036854775807 -1) because the q > (double)INT64_MAX guard can never fire. Integer operands now divide in int64 space with a floor correction, matching the temporal mod path; the double path is kept only for float operands with a tightened guard. * fix(expr): keep compiled integer div exact * perf(expr): fast-path integer floor-div within ±2^53 to the double kernel Keeping every integer OP_IDIV on the exact scalar int64 kernel (PR #403) made the hot columnar div path ~2x slower: hardware 64-bit idiv is scalar and replaced the vectorizable divsd+floor. Restore the fast path without losing exactness — per morsel, when both operands are within ±2^53 (a cheap scan, over the in-cache morsel in the fused kernels) delegate to the vectorizable double loop, which is bit-exact there (round(a/b) cannot cross an integer boundary in that range); fall back to the exact int64 kernel only for large magnitudes or nulls. Applied to the fused null-aware and non-null I64 kernels and all four binary_range output arms. Measured (cache-proof, distinct constant divisors, 20M rows ×10 iters, release): exact-only 0.41s -> dual-path 0.24s. Also folds in the review's smaller points: - ray_idiv_fn guards INT64_MIN/-1 in the integer path (symmetry with floor_idiv_i64_checked; unreachable today but removes latent UB); - the exact narrow I32/I16/U8 arms saturate to match their ray_cast_f64_to_iN_null double counterparts instead of wrapping; - binary_range's I64 OP_DIV arm uses floor_idiv_i64_checked, not the open-coded form; - 0x1p63 documents the scalar float-cast bound. Extends test/rfl/integration/math.rfl: fast/exact boundary agreement at 2^53, mixed small+big morsels, div-by-zero through the integer path, and narrow-output floor div. --------- Co-authored-by: Anton Kundenko <singaraiona@gmail.com>
…ollow-up) PR #403 added a ±2^53 magnitude gate so integer `div` keeps the vectorizable double kernel for small values. The gate itself was cheap in principle but paid for twice over in binary_range: it walked BOTH operands in a full extra pass, reading every element through the 6-way LV_READ_I64/RV_READ_I64 ternary chain — including a broadcast SCALAR divisor, whose single value was re-read n times. `select (div a 7)` over 20M rows went 107 -> 164 ms (min-of-10, -c 1). Scan plumbing only — the predicate, the exact int64 kernel, the gate constant and every result are untouched: - a broadcast scalar operand is range-checked once, outside any loop; - a narrow vector (I32/U32/I16/BOOL/U8) cannot leave ±2^53, so it is not scanned at all; - an I64 vector is scanned through its typed data pointer, and two I64 vectors share a single pass (i64_span2_dbl_exact); - for the hot `I64 column ÷ (I64 column | int scalar)` shapes with I64 output there is no pre-pass: the range check rides inside the divide loop (values already in registers) and the range is recomputed by the exact kernel only if the flag fires — the same all-or-nothing decision, minus a DRAM-bound second read of the column. - the fused kernel's two span scans are likewise fused into one. Span scans are now branchless accumulations so the passing (common) case stays auto-vectorized. Measured (release, 20M rows, min-of-10, -c 1 | -c 8): base 2ec5284 merged 66a80fb this div a 7 107 | 31 164 | 45 65 | 24 div a b 91 | 42 114 | 43 94 | 42 * a b (ctl) 40 | 30 40 | 30 40 | 30 Verified byte-identical to 66a80fb on a div corpus (all sign combos, nulls, 2^53±1 both signs, INT64_MIN-adjacent, scalar and vector divisors, narrow widths, timestamp column) at -c 1/2/4/8. math.rfl pins the fast lane, the redo lane and the scalar-divisor lane of the inline gate.
…der the inline gate
clang's -Wbitwise-instead-of-logical (fatal under -Werror on macOS CI) rejects `bad |= !x | !y`; split into two int|=bool statements — same branchless accumulation, no warning on either compiler.
Rayforce targeted audit passedThe required Rayforce audit gate passed on the latest run. Workflow run: https://github.com/RayforceDB/rayforce/actions/runs/31979725996 |
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
4 participants
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.
What & why
Checklist
dev(notmaster)feat:/fix:/perf:/docs:/ …)makebuilds cleanly (no new warnings)make testpasses; tests added/updated for behaviour changes