v2.8.0 - #547
Merged
Merged
v2.8.0#547
Conversation
collect_part_dirs sorted every partition scheme with strcmp. That is the key order for dates (fixed-width YYYY.MM.DD) and for opaque names, but an all-integer set came out in string order — 1, 10, 2 — and every consumer takes that order as the key order: ray_read_parted emits the I64 MAPCOMMON keys in it, so the partition-key column of an integer-partitioned table was unordered and rows interleaved across keys; ray_parted_tables and ray_parted_fill read the "most recent" partition as the last one, so for partitions 1..12 that was "9" and a table first added in 12 was invisible to .db.parted.tables and never backfilled. Sort an all-integer set by value. The check mirrors infer_mc_type, so the set sorts numerically exactly when it is classified RAY_MC_I64; date and symbol sets sort as before. parse_int_dir also accumulated the digits with no bound: a 20-digit name overflowed int64 — UB that wrapped the key to an unrelated number, and an abort under UBSan (part.c:109). Parse with an overflow check; a name whose value does not fit int64 fails integer classification, so the set drops to symbol partitioning with the name kept literal — the treatment #492 gave an impossible calendar date. test_eval_insert_parted_key_types pinned the string order ("10","2" load as [10,2], so any insert was 'corrupt'); it now checks the keys load as [2,10] and that the immutable-tail contract holds on them: a later key appends, the last key grows, an earlier key is rejected as historical.
Address review on #530. - A digit-only directory name that does not fit int64 in an otherwise integer set is now a `corrupt` error naming the directory, from every reader of the set (.db.parted.get / .tables / .fill), instead of a silent retype of the whole set to SYM keys — where (== part 2) matched nothing and (>= part 2) returned partition 1 too. A mixed set (digit-only names beside dotted non-date names) is still SYM with the long name kept literal: there every name is an opaque key. - Equal-valued spellings (01 / 1) sort by name after the value, so the order is filesystem-independent again. - collect_part_dirs classifies the set once through infer_mc_type and hands the key type to ray_read_parted; integer names parse through ray_parse_i64 (overflow-checked, unit-tested, fuzzed); the O(n^2) exchange sort is a qsort with a comparator; the unreachable minus-sign path is gone. collect_part_dirs returns the error object itself so the corrupt case can name the directory; io/oom messages are unchanged. - Tests: restore coverage of the insert validator's "keys are not strictly increasing" branch with a 2 / 02 fixture; cover the corrupt error naming the directory, the mixed-set fallback, and INT64_MAX / INT64_MAX + 1. - Docs: db.md and functions.md no longer tell users to zero-pad integer directory names; .db.parted.get documents how names pick the key type.
…r gather dispatch (#531) Every query graph node, ext node, const vector and optimizer scratch buffer was allocated with ray_sys_alloc, which is a dedicated mmap per call and a munmap (plus TLB shootdown) when the graph is freed at the end of the query. A point lookup on a 4-row table cost ~10 mmap/munmap pairs and ~49 us. ray_sys_alloc is meant for infrastructure that cannot use the buddy heap (cross-thread lifetime, bootstrap, global state); graph.c predates ray_alloc_raw and was never migrated. Move graph.c, opt.c, idiom.c, query.c, collection.c, rerank.c, embedding.c, datalog.c and builtins.c per-call buffers onto ray_alloc_raw / ray_calloc_raw / ray_realloc_raw / ray_free_raw. Buffers that are fully written before being read (the node array, DFS stacks, index scratch) take the uninitialised form; flag arrays keep their memset or use the zeroing form, and memsets that duplicated it are dropped. sel_compact and exec_filter dispatched the multi-column gather to the pool for any pass_count, waking every worker to copy one row (~6 futex per query). Gate both on ray_pool_par_dispatch_ok(pool, pass_count, RAY_PARALLEL_THRESHOLD) like the other pool users. Reporter's workload, 4-row table, 50000 point selects, -c 0: 49.5 us/query -> 3.3 us/query; 120k mmap + 120k munmap + 120k futex -> 49 + 28 + 20 for the whole run.
fix(store): order integer partitions by value; reject names past int64
perf(engine): per-query scratch on the buddy heap, gated filter gather dispatch (#531)
Add aggregation admission diagnostics and execution-route coverage for the P0 type-support plan. Fix grouped BOOL/F32 median and quantile, BOOL output strides, and unaligned radix row-position access. Record the implementation plan and baseline census. Validation: 3794 full ASan/UBSan tests passed, final contract tests passed with one- and two-core harness settings, and release build and profiler smoke checks passed.
#533) expr_compile called ray_vec_has_nulls on every SYM column it compiled. Text metadata is deliberately untrusted, so that walked every row through an out-of-line ray_vec_is_null call, on every query, before the predicate ran. A point lookup on a 100k-row SYM column paid ~11x the I64 cost, all of it in the null proof. Two changes: * expr.c: a pre-pass over the compiled subtree marks SYM scans whose every consumer is EQ..GE against a non-null constant or another SYM scan. A null SYM is id 0, below every real intern id, and the fallback ranks a null SYM below everything in all six comparisons, so raw id compares in the fused lane already produce the null-aware answers. Those columns skip the proof and compile as non-nullable. Consumers that read the lane as a value (ISNULL, CAST, arithmetic) keep the scan and still bail on payload nulls. * vec.c: the remaining callers of ray_vec_has_nulls on SYM/STR get a width-specific chunked zero scan instead of the per-row call, ~8x cheaper with identical results. Point lookup, release build, single thread, 100k rows: SYM 475us -> 43us, now equal to the I64 filter. Tests: fused-vs-fallback diff over all six comparisons, column-vs-column, AND/OR trees and a parted SYM column with nulls at three widths; value consumers assert EXPR_BAIL_NULLS; ray_vec_has_nulls pinned at every SYM width, on STR and through slices. text_null/sym_expr_admission updated to the new contract.
The null-aware kernels rank a null below every value: null != c, null < c and null <= c are TRUE. Chunk-zone extrema exclude nulls, so deciding a morsel all-fail from extrema alone dropped those rows whenever the chunk also held a null. Guard the NE/LT/LE all-fail arms on the chunk's null bit, in expr.c's zone_cmp_decision and in the matching arms of fused_pred.c's fp_eval_cmp. The fused_pred arms are currently unreachable with nulls because fp_col_supported_op rejects nullable numeric columns; a tripwire test pins that rejection so lifting it also brings zone coverage. Test: 4096-row nullable I64 column with a 1024-row chunk-zone index so each morsel is one chunk, nulls inside two all-equal chunks, all six comparisons checked per row through ray_execute.
…llable numerics dev's typed predicate path admits nullable numeric columns, so the tripwire asserting rejection failed on the CI merge build. Replace it with the per-row check through fp_eval_pred, one chunk per call.
perf(expr): drop the per-query SYM null scan for null-safe comparisons (#533)
call_lambda binds `self` for a tree-walked call under a sym ID it cached in
a function-local static on first use and never cleared. Runtime destroy
tears the sym table down, and the next runtime can intern "self" at a
different ID. From then on every tree-walked call — every call to a
closure, which is never compiled — bound the lambda under the old ID:
`self` in the body no longer named the call's lambda, and whatever
ordinary name had taken the old ID was shadowed by the lambda.
Seen as an upsert inside a closure called through map receiving the
closure itself as its key argument ("key must be a symbol or integer,
got ?"), and only after an earlier runtime in the same process had cached
the ID — in the test runner, after compile/let_reserved_name.
Move the ID to file scope and clear it in ray_lang_destroy next to
ray_compile_reset, which already does this for the compiler's special-form
IDs (env.c resets its own `self` cache in ray_env_destroy).
Test: runtime/lambda_self_sym_per_runtime creates two runtimes in turn,
padding the sym table by different amounts so "self" gets a different ID
in each, and runs a closure that recurses through `self` and returns a
captured value. Without the fix the second runtime fails with a
stack-limit error; one cached ID cannot match both runtimes, so the test
fails however the cache was filled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#535) A multi-row `upsert` boxed every payload cell into an atom, type-checked and hashed through those atoms, and wrote each matched cell by appending it to the column and copying the new tail into place — one append, one copy and one index drop per cell. For a batch of 935 rows over 6 columns that is 5610 allocations and 5610 tail copies per call. When the payload is a table (or a row of vectors) whose columns are already the target's own types, none of that is needed. upsert_apply_vectors resolves the batch in payload order through the same key map, appends the new rows with one ray_vec_append_raw per column, and scatters the matched cells in payload order — dropping the index and clearing SORTED once per column instead of once per cell. Admission (upsert_vec_payload_ok) requires every target column to have a payload vector of its exact type: BOOL, U8, I16, I32, I64, F64, DATE, TIME, TIMESTAMP, GUID, or SYM already in the runtime domain. STR and LIST columns, partial payloads, width coercions and slices keep the per-cell arm. Observable behaviour is unchanged: a new key enters the map at the row it will take, so a key repeated inside the batch still ends at its last value, and the appends run before the scatter because a repeat can target a row the same batch just appended. Nulls travel as their sentinels and raise HAS_NULLS on the target column. On a table past the per-core cache nearly all of a probe is two misses — the slot, then the key cells of the row it names — and one row at a time they never overlap. A second instance of the resolve loop hashes the batch first and loads both a few rows ahead; upsert_apply_vectors picks it when the slots plus the key columns exceed UPSERT_PF_MIN_BYTES, and the scatter picks the same look-ahead per column by the column's own size. Both loops exist twice with the flag a compile-time constant, and the per-row probe helpers are force-inlined: a runtime flag inside the loops, or the calls GCC left behind once the loop existed twice, each cost more on cached-sized tables than the prefetching gained. Test: test/rfl/table/upsert_vector.rfl applies every batch twice — as a vector payload and row by row as atoms — and requires identical tables, over the order-book shape, every admitted value and key type, keys already duplicated in the target, null and NaN keys, the `(list …)` row form, a shared target, a value target, hash indexes on key and non-key columns, the sorted attribute, and tables past the look-ahead threshold (compared as serialized bytes). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the slots
The table-resident key map stored `row + 1` in a slot and nothing else, so it
could only ever be replaced: a batch that no longer fit the load factor dropped
the map and rebuilt it by reading every key column again and re-hashing every
row. That rebuild is the longest pause a growing table pays — measured here at
90 ms for 1M rows with a three-column key, because the writes land at random in
a slot array well past the last level of cache.
sym.c has solved this for the intern table since the beginning: its buckets
hold `(hash32 << 32) | (id + 1)`, so ht_grow_to fills the bigger table from the
old buckets alone. Do the same here. A slot now packs the low 32 bits of the
key's hash in its high half and `row + 1` in the low half, and ukey_grow
re-places every live entry straight from the old slots — no key column is read,
no key is hashed again, and the tombstones a delete left behind are dropped on
the way. ukey_fits splits into ukey_describes (this map still names this
table's rows and key) and ukey_has_room; only the second failure becomes a
growth, the first stays a drop, since a map that stopped describing the table
cannot be re-placed into a correct one.
The stored hash pays a second time on every probe: a colliding slot is rejected
by comparing 32 bits instead of reading the candidate row's key cells, which is
what makes the plain update path faster too.
The packing reserves both markers (an occupied slot is never 0 and never
all-ones) and bounds the map to 2^32 slots, so a table at or past 2^32 rows
simply gets no map and keeps the previous per-call behaviour.
Measured against the same branch without this commit, batch of 935 rows,
medians of five interleaved runs, ms per batch:
insert 360k rows 0.373 -> 0.161
insert 1M rows 0.853 -> 0.332
insert 1.2M rows 0.060 -> 0.047
update 360k rows 0.081 -> 0.073
update 1M rows 0.107 -> 0.085
The batch that actually crosses the capacity, timed on its own: 22.98 -> 16.84
ms at 600k rows, 193.5 -> 86.4 ms at 1M. The rebuild a foreign write still
forces (a map that no longer describes the table) keeps its old shape and
measured 90 -> 70 ms at 1M.
Test: upsert_vector.rfl grows the map twice — once past its capacity and once
after a delete has left tombstones — and compares against the per-cell path as
every other case does, including a key the delete removed being appended again
through the grown map.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
perf(query): write a keyed upsert of a vector payload column by column (#535)
fix(eval): reset call_lambda's cached `self` sym ID with the runtime
The map enters rows in ascending row order, so two rows carrying the same key sit in their probe chain lowest-row first and a keyed upsert updates the lowest one — the row the linear scan it replaced would have picked. ukey_grow (#537) re-placed the live entries by walking the old table from slot 0, which keeps that order only while a chain lies in one piece: a chain that wraps past the end of the array is met tail first, so the two entries swap and the next upsert updates the later row. Walk the old table from an empty slot instead, cyclically. A chain never contains an empty slot, so every chain is then traversed from its beginning, and the load factor of at most a half guarantees such a slot exists. Seen with a table of 480 rows whose key 470 is duplicated by an insert: after the map grows, the upsert of key 470 lands on the appended row instead of row 470. Test: upsert_vector.rfl pins the duplicate resolution across a growth for the sizes that expose it, against the per-cell path as every other case is. The RAY_IDX_UKEY comment gets the slot layout #537 changed, and the ordering guarantee this restores. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(query): re-place the upsert key map from a chain start when it grows
…e swap The American-flag pass moved records inside a bucket with an in-place swap cycle, which is what shuffled equal strings in the first place; the previous commit repaired the damage afterwards by re-sorting every bucket of equal strings on (len, row), and found descending runs with a full string compare per neighbour. Scatter each byte pass through a scratch slice in scan order instead, so equal strings never leave their source order: the bucket of bitwise-equal strings needs no re-sort again, and the descending run walk can reject neighbours on length before it touches bytes. The scratch is one buffer of n_live records: sequential sorts allocate it, parallel sorts reuse the top-level scatter's drained source, so peak memory does not change for the parallel path and grows by one key array for the sequential one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Issue #551. #549 made compression a per-link decision, but the multicast fan-out frame is shared by every subscriber of a topic (#487), so publish still framed at the compiled-in RAY_IPC_COMPRESS_THRESHOLD: a tickerplant with only loopback subscribers compressed on every publication and every subscriber decompressed it. ray_ipc_frame_async_at frames at an explicit threshold; ray_ipc_frame_async keeps its signature and passes the default. ray_mcast_pub groups a topic's subscribers by the threshold their link reports (ray_ipc_handle_threshold, from #549) and builds one framing per distinct value, lazily on first use. An all-local topic therefore builds exactly one framing and never compresses it, preserving #487's single serialization per publication; a mixed topic pays one extra serialization rather than one per subscriber. MC_MAX_FRAMINGS caps a publication at 4 distinct policies; past that everyone shares the first framing, which is correct for any peer because the receiver honours the per-frame COMPRESSED flag — verified in #549 by running a never-compress build against a compressing one in both directions. mc->framed counts framings built rather than publications, which is what makes the bucketing observable in .mc.stats. Tests: ipc/mcast_local_not_compressed subscribes a raw socket, publishes a 5000-element i64 run (well past the threshold and highly compressible), and asserts on the delivered frame's header that RAY_IPC_FLAG_COMPRESSED is clear. Verified to fail against dev's publish path with `got 1, expected 0` and to pass with this change, so it pins the behaviour rather than the implementation. It also asserts .mc.pub reached exactly one subscriber, so the header assertions cannot pass vacuously. ipc/frame_async_at_threshold covers the framing primitive directly: the same payload frames compressed at the default and raw at NEVER, asserted on both the flags and the resulting frame size. make test 3878/3878, no ASan/UBSan reports.
Closes #555. pearson_final_result computed `num / sqrt(dx*dy)` with no guard, relying on the comment's claim that "a constant side → sqrt(<=0)" produces NaN which the single-null float model canonicalises to NULL_F64. That holds only when exactly ONE of dx/dy is negative. dx and dy are >= 0 mathematically (Cauchy-Schwarz), but a constant column cancels to a small NEGATIVE residue in doubles, so with BOTH sides constant the product is positive, the root is finite, and a finite "correlation" outside [-1, 1] is emitted: pearson_corr(x, x) x constant -> -1.0 pearson_corr(c1, c2) both const -> 2.13 Test dx and dy separately and return NULL_F64 when either is non-positive, which is what the legacy keyed path already does (src/ops/group.c) — so this restores parity rather than inventing a rule. n < 2 leaves dx = dy = 0, so the same guard covers it and the existing single-row behaviour is unchanged. cov / scov / wavg were checked for the same shape and do not have it: covariance is unbounded so its residue is imprecision rather than an out-of-range value, and scov and wavg already guard n <= 1 and sx == 0. test/rfl/agg/pearson_degenerate.rfl pins both directions: the degenerate shapes come back null, and perfectly correlated and anti-correlated data still return exactly 1.0 and -1.0. make test 3877/3877, no ASan/UBSan reports.
…ying on it Closes #556. agg_ord_compact_fn compacts `pairs` in place. Its safety comment proves that an EARLIER chunk's writes cannot clobber a LATER chunk's unread input, but the reverse is the case that actually arises: a later chunk writes at its prefix offset, which sits far below its own input range whenever groups are sparse relative to rows — any ordinary group-by with duplicates. At 10M rows / 100k groups, prefix[76] is about 98,000 while chunk 0 is still scanning [0, 131072). That is safe today only because ray_pool_dispatch's grain is 8192 ELEMENTS and the dispatch extent here is n_chunks, so the compaction has always been a single task running its chunks in ascending order — the forward-safe order the comment proves. Above 2^30 rows n_chunks would exceed the grain, the compaction would split into concurrent tasks, and a late task could overwrite an early task's unread region: one group silently duplicated and another dropped, with the row count still correct so the `ordered == ng` check cannot see it. Gate the parallel path on n_chunks fitting in one task instead of depending on it, so the code relies on the invariant its comment actually establishes. Above that it runs serially — which is what it already did in every reachable case, so nothing gets slower. Not reproducible: the racing path needs an input larger than 1,073,741,824 rows, so this is a fix without a failing test. The bound comes from reading ray_pool_dispatch's grain, not from measurement.
Closes #554. The grouped accumulator computed variance in one pass as sumsq/cnt - mean*mean. Once values reach ~1e9 those two terms agree to within double precision and the subtraction cancels; the `vp < 0 ? 0` clamp then turned the negative residue into a clean 0.0, so the wrong answer looked like a plausible low-variance result rather than a failure. Variance is translation-invariant, so the bug is visible without any reference implementation: the same spread shifted by a constant gave 2.0 at base 0 and 0.0 at base 2e9, while the scalar builtin — which uses a stable two-pass algorithm — returned the right answer throughout. The integer accumulators additionally kept the sum of squares in a wrapping int64, so TIMESTAMP (~8e17 ns) overflowed on the first row and I32 near 2^31 after two. Note that fixing only the wrapping would not have helped: F64 already carried a double sum of squares and still returned 0.0 at 2e9, because the cancellation is the actual defect. Both families now accumulate shifted data: sums are of (v - k), where k is the first value a state sees. Centring keeps both terms small whatever the magnitude, so the final subtraction cannot cancel, and the result is translation-invariant as the definition requires. Merging two states re-centres one on the other's origin: sum' = sum + n*dk sumsq' = sumsq + 2*dk*sum + n*dk^2 (dk = k_src - k_dst) Integer inputs take the difference in INTEGER arithmetic before converting. A TIMESTAMP near 8e17 has a double ULP of 128, so converting first quantises every value to 128 ns and leaves a relative error around 1e-7 — enough to fail the translation-invariance test on its own. The subtraction is done on unsigned so it is wrap-defined; for any span under 2^63 it is exact. State grows from 24 to 32 bytes per group for the ten variance vtables (k is added); agg_vo_init's layout needs every state_size to be a multiple of 8, which 32 is. test/rfl/agg/variance_large_magnitude.rfl uses the invariance itself as the oracle, so it needs no reference implementation: the same spread at base 0, 1e6 and 2e9 must agree, for F64, I32, I64 and TIMESTAMP, and a zero-variance group must still be exactly 0.0. It fails on dev at the 1e6 case, so the imprecision starts well below the 2e9 that made it obvious. Performance is unchanged: 5M rows / 1000 groups, best of 5 at -c 8, F64 var 5.31 -> 5.38 ms and I64 var 5.42 -> 5.43 ms, with an avg control moving -1.6% in the same runs. make test 3877/3877, no ASan/UBSan reports.
…cess Audit finding on #557. The framing-failure branch I added was unreachable: ferr is set only when mc_frame_for returns NULL, and every NULL return went through the drop path which increments dead_n, so the guard's `dead_n == 0` conjunct could never hold. The live behaviour was therefore worse than the base: an allocation failure inside ray_ipc_frame_async_at dropped every affected subscriber, CLOSED its connection, and still returned ray_i64(seq) — a normal success value — where ray_mcast_pub previously returned an error and left all subscriptions intact. A framing failure is a local resource failure and says nothing about any peer, so treating it like a failed send was wrong in the first place. Build every framing the topic needs BEFORE sending any of them. Nothing has been delivered when the failure is detected, so the error path leaves every subscription exactly as it was and the publication is all-or- nothing rather than half fanned out. The extra pass over the subscriber array is cheap and the framings themselves are still built lazily, one per distinct threshold. Also from the same audit: - the `framed` field comment still said "one per publication, not per subscriber"; it is now one per distinct compression policy per publication. - the new test claimed .mc.pub returns the number of subscribers it reached. It returns the topic's SEQUENCE number, so the assertion passed because next_seq starts at 1, not because anything fanned out. What actually rules out a vacuous pass is the read that follows: with no publication the socket never becomes readable and the test fails there. Comment corrected and the misleading equality dropped. make test 3878/3878, no ASan/UBSan reports.
fix(query): don't deref a declined upsert key map; correct agg-v2 doc
fix(agg): keep the first-seen order compaction serial rather than relying on it
fix(agg): pearson_corr must be null when a side has no variance
fix(pool): run dispatch_n past the task ring in rounds instead of dropping tasks
feat(mcast): one framing per distinct subscriber compression policy
fix(sort,pivot): stable sort on every path; pivot output in first-seen order
fix(agg): numerically stable grouped variance and stddev
Closes #561, reported by @vbmithr as the encode-side counterpart of #542. ray_sym_str takes the global sym spinlock around a single array read, so walking a SYM column cost one atomic exchange per cell — and both ray_serde_size and ray_ser_raw walk it, so a frame paid 2 * rows * cols lock round-trips for a handful of distinct strings. On a single-threaded tickerplant that spinning is entirely uncontended. ray_str_len and ray_str_ptr are out-of-line calls made per cell on top of that, and profiled at 30% of encode on their own. A direct-mapped id -> (bytes, len) cache in thread-local storage avoids all three on a hit. Sym ids are stable positions for the lifetime of the table, so the cache persists across calls and needs no per-column setup; the new ray_sym_epoch changes when the table is torn down and re-inited, which is the only event that can invalidate it. Encode allocates nothing, as it did before this change. A column with far more distinct symbols than slots would otherwise pay the probe and the store on top of a resolve it still has to do: measured 15% SLOWER than not caching. So the first SYM_ENC_SAMPLE cells are sampled and, if fewer than a quarter hit, the remainder of the column falls through to the original loop verbatim — routing the cold path through a shared helper instead cost another 7% in ray_ser_raw, which the profile showed plainly. Both numbers are a heuristic, not a limit: either way every cell resolves correctly. Non-runtime domains already resolve lock-free and the RAY_SYM_AUDIT hook is a per-cell contract, so both skip the cache entirely. Measured on this machine, release builds: 914-row depth batch, 4 SYM columns 90.6 -> 35.2 us (-61%) same shape, SYM as I64 (control) 6.6 -> 6.6 us 20k rows, 3 distinct symbols 410.2 -> 128.1 us (-69%) 20k rows, 20k distinct (thrashing) 462.8 -> 473.9 us (+2.4%) IPC publisher, 20k frames 147.0 -> 46.0 us/frame (-69%) IPC subscriber 73.0 -> 69.5 us/frame bytes on the wire 65875 -> 65875 (identical) Every benchmark run applied all 18,280,000 rows. The residual +2.4% on a column where every row is a distinct symbol is the bounded sample prefix; market-data SYM columns are the opposite shape. The wire format does not change, which the existing SYM round-trip tests cover: the cache is only for resolution, so a column of 12 cells with 2 distinct values still encodes 12 strings. make test 3884/3884, no ASan/UBSan reports.
perf(serde): resolve a SYM column's distinct symbols once per encode
try_count_distinct_v2_rewrite recognised the group key only as a bare
symbol (one key) or a {alias: col ...} dict (composite with aliases). A
composite key written the ordinary way, `by: [MobilePhone MobilePhoneModel]`,
matched neither, so `count (distinct x)` over such a key skipped the fused
kernel and the two-pass rewrite and ran through the general per-group path:
one exec_count_distinct per group with per-atom hashing, half its time in
the allocator. The same query in dict form took a seventh of the time.
Accept a symbol vector as the list of key columns; the output columns keep
the source names, which is what that form means everywhere else.
300k rows, 50 x 300 SYM key pairs, 17% distinct values, warm ms:
by: [a b] 85.0 -> 11.4
by: {a: a b: b} 12.0 -> 12.1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t …), not a mixed vector literal
…t symbol A `by:` expression that reads exactly one flat SYM column (substr / str-find / if over it) was compiled into the DAG and run over every row, and every string op over SYM interns its output per row — eight passes of interning for a host-extraction key. Such a key is a function of the symbol alone: collect the distinct ids actually present (first-seen slot table), evaluate the expression through the same DAG compiler over that distinct vector (a one-column table adopting the column's domain), and spread the result over the rows by slot. The spread key feeds the group node as a constant; since a const node's ext slot holds the literal, not a name, the key column is named after execution the way the eval-level path names a computed key (last bare symbol of the form, else `key`). Gates: >= 4096 rows, distinct <= rows/2, domain <= 16x rows and <= 64M; any id outside the domain (nulls) or a second column reference falls back to the row-wise evaluation. The eval-level computed-key path uses the same helper. Test group/derived_key_sym_domain: result vs an oracle that materialises the key as a column and groups on it — incl. where, a key that keeps the column's own symbols, an all-distinct column (fallback), nulls, and a two-column expression (must not take the shortcut). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…a near-unique column The first-seen slot pass used to give up only once the distinct count passed rows/2 — half a pass of random slot writes over a domain-sized table on an all-distinct column. Now, after the first 65536 rows, a distinct share above 3/4 aborts the pass; the row-wise key path follows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… path; name a computed key consistently The per-symbol evaluation is valid only for a key that is a pure, row-local function of the symbol. The gate was syntactic (one column referenced), so positional operators — differ, fills, deltas, moving windows — evaluated over the distinct symbols instead of the rows and grouped silently wrong (`by: (differ ref)` collapsed to one group), and a free name bound to a table-length global vector broke the same way. The key expression now has to pass an allowlist walk: heads from a fixed set of elementwise operators (let/if/cond, logic, comparison, arithmetic, substr/str-find/strlen/within/nil?/upper/lower/like/as/ xbar), let-scoped names, symbol literals and the column itself; anything else falls back to the row-wise key. A single computed key on the DAG path took its column name from its op's ext sym, which is not a name (an expression's ext resolves to whatever shares its id, a const node's slot holds the literal), so the same query named the key `null` on the row-wise path and by the by-form on the per-symbol path. The key column is now named by the eval-level rule (last bare symbol of the form, else `key`) whichever path ran. The null symbol is id 0, inside the domain: it gets a slot like any other symbol; the comment said otherwise. Tests: differ / fills / a global-vector predicate against the row-wise oracle; the key column name at 20k rows and below the 4096-row gate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…p the key name off an output alias - The near-unique probe over the first 65536 rows now runs through a 128k-slot scratch set before any domain-sized allocation; a query that falls back no longer pays a dn-sized slot table and its memset. - A quoted symbol that names a column of the table is resolved to that column by the DAG compiler, so such a key is not a function of the one symbol; the allowlist walk rejects it. - A computed key's derived name yields to an output alias spelling the same (`by: (substr ref 0 2) ref: (min ref)` → `[key ref]`), and the rename waits for a lazy result to materialise. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ps its name The rename flag was set for the whole single-key branch, which a bare `by: k` also takes (compiled as a scan), so every plain one-key group came back with its key column called `key`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ter, right-sized probe - A head name is admitted only while it still means the builtin: the DAG compiler inlines a global lambda of that name ahead of the builtin, and a let-bound name shadows it, so both reject the per-symbol evaluation. - agg_route_stats().key_domain_evals counts keys evaluated once per distinct symbol, so a test can tell the path fired instead of relying on an oracle that both paths satisfy. test_agg_contract: substr and a let/str-find/if key fire once; differ, a two-column key, a table below the row gate and a shadowed substr do not. - The near-unique probe set is sized to the probed rows (power of two, load <= 1/2, at least 1024 slots) instead of a fixed 1 MiB. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The near-unique probe hashed into a fixed 17-bit range but the set is now sized to the probed rows, so the first access could land past the end of a smaller set (4k-64k-row tables). The initial index is masked like the collision step. The first-seen slot counter is int32; the row/2 gate is capped to match. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
perf(select): evaluate a single-SYM-column group key once per distinct symbol
fix(query): admit a symbol-vector by: to the count-distinct rewrite
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
3 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