diff --git a/src/ops/collection.c b/src/ops/collection.c index 6b20c4b24..b61bc3f17 100644 --- a/src/ops/collection.c +++ b/src/ops/collection.c @@ -65,13 +65,60 @@ typedef struct hashset_t { int8_t src_type; /* ray_t.type */ bool src_has_nulls; void* src_data; /* pointer to typed data (or RAY_LIST elements) */ + /* Int-family cells hash through f64 (hs_hash_row). Off until a probe + * crosses numeric classes — see hs_needs_num_f64. */ + bool num_f64; + int8_t probe_type; /* last probe type vetted by hashset_adopt_probe */ } hashset_t; +/* Numeric class of a typed-vec type, matching is_numeric/atom_eq: every + * int width compares to every float width through f64. Temporal types are + * not numeric there (a DATE never equals an I32), so they stay class 0. */ +enum { HS_NUM_NONE = 0, HS_NUM_INT = 1, HS_NUM_FLT = 2 }; +static inline int hs_num_class(int8_t t) { + switch (t) { + case RAY_BOOL: case RAY_U8: case RAY_I16: case RAY_I32: case RAY_I64: + return HS_NUM_INT; + case RAY_F32: case RAY_F64: + return HS_NUM_FLT; + default: + return HS_NUM_NONE; + } +} + +/* Cell i of a numeric typed vec as f64 — as_f64 on the boxed atom. */ +static inline double hs_cell_f64(int8_t t, const void* data, int64_t i) { + switch (t) { + case RAY_BOOL: return (double)((const bool*)data)[i]; + case RAY_U8: return (double)((const uint8_t*)data)[i]; + case RAY_I16: return (double)((const int16_t*)data)[i]; + case RAY_I32: return (double)((const int32_t*)data)[i]; + case RAY_I64: return (double)((const int64_t*)data)[i]; + case RAY_F32: return (double)((const float*)data)[i]; + default: return ((const double*)data)[i]; + } +} + +/* Int cells hash with ray_hash_i64 and float cells (typed, or numeric atoms + * in a RAY_LIST) with ray_hash_f64, so 3 and 3.0 land in different buckets + * and hs_eq_rows never sees them (#645). An int side meeting a float or + * list side must hash its ints through f64 as well. */ +static inline bool hs_needs_num_f64(int8_t a, int8_t b) { + int ca = hs_num_class(a), cb = hs_num_class(b); + if (ca == HS_NUM_INT) return cb == HS_NUM_FLT || b == RAY_LIST; + if (cb == HS_NUM_INT) return ca == HS_NUM_FLT || a == RAY_LIST; + return false; +} + /* Hash a single row at index i in src. Mirrors atom_eq's coercion * rules: numeric types normalize through f64 so an I64 atom and an - * F64 atom holding the same value collide (boxed-list path only — a - * typed vec is homogeneous, so the dispatch picks one branch). */ -static uint64_t hs_hash_row(ray_t* src, int64_t i, int8_t t, void* data) { + * F64 atom holding the same value collide. A typed int vec hashes + * through i64 unless num_f64 is set — the set is being probed from the + * other numeric class (hs_needs_num_f64). */ +static uint64_t hs_hash_row(ray_t* src, int64_t i, int8_t t, void* data, + bool num_f64) { + if (num_f64 && hs_num_class(t) == HS_NUM_INT) + return ray_hash_f64(hs_cell_f64(t, data, i)); switch (t) { case RAY_I64: return ray_hash_i64(((const int64_t*)data)[i]); case RAY_I32: return ray_hash_i64((int64_t)((const int32_t*)data)[i]); @@ -169,6 +216,10 @@ static int hs_eq_rows(ray_t* a_src, int64_t ai, int8_t at, void* a_data, } } } + /* Mixed numeric typed vecs: atom_eq compares any two numerics + * through f64 — do the same without boxing a pair per probe. */ + if (hs_num_class(at) != HS_NUM_NONE && hs_num_class(bt) != HS_NUM_NONE) + return hs_cell_f64(at, a_data, ai) == hs_cell_f64(bt, b_data, bi); /* Fall back to atom_eq via boxed values. Used for cross-type * comparisons (e.g. except over typed I64 vs F64 vec) and the * RAY_LIST path. collection_elem allocates a temporary atom for @@ -208,6 +259,8 @@ static bool hashset_init(hashset_t* hs, ray_t* src, int64_t hint) { hs->src_type = src ? src->type : 0; hs->src_has_nulls = src ? ray_vec_may_have_nulls(src) : false; hs->src_data = src ? ray_data(src) : NULL; + hs->num_f64 = false; + hs->probe_type = hs->src_type; return true; } @@ -216,11 +269,11 @@ static void hashset_destroy(hashset_t* hs) { hs->slots = NULL; } -static bool hashset_grow(hashset_t* hs) { +/* Re-slot every stored row into a fresh table of new_cap, hashing with + * the set's current num_f64 mode. */ +static bool hashset_rehash(hashset_t* hs, int64_t new_cap) { int64_t old_cap = hs->cap; int64_t* old_slots = hs->slots; - int64_t new_cap = old_cap * 2; - if (new_cap < old_cap) return false; ray_t* nb = ray_alloc((size_t)new_cap * sizeof(int64_t)); if (!nb || RAY_IS_ERR(nb)) return false; int64_t* ns = (int64_t*)ray_data(nb); @@ -229,7 +282,8 @@ static bool hashset_grow(hashset_t* hs) { for (int64_t i = 0; i < old_cap; i++) { int64_t ridx = old_slots[i]; if (ridx == HS_EMPTY) continue; - uint64_t h = hs_hash_row(hs->src, ridx, hs->src_type, hs->src_data); + uint64_t h = hs_hash_row(hs->src, ridx, hs->src_type, hs->src_data, + hs->num_f64); int64_t s = (int64_t)(h & (uint64_t)mask); while (ns[s] != HS_EMPTY) s = (s + 1) & mask; ns[s] = ridx; @@ -242,8 +296,50 @@ static bool hashset_grow(hashset_t* hs) { return true; } +static bool hashset_grow(hashset_t* hs) { + int64_t new_cap = hs->cap * 2; + if (new_cap < hs->cap) return false; + return hashset_rehash(hs, new_cap); +} + /* Probe the set for the row (probe_src, probe_i). Returns the stored * row index from the build-side vec on hit, HS_EMPTY on miss. */ +/* A probe of a type the set has not seen yet. From the other numeric + * class, rehash once so int cells hash through f64 — later probes and + * inserts keep the mode, and same-type callers never get here. Returns + * false on OOM: the slots keep their old layout, the type stays unvetted + * so the next probe retries, and the caller answers by hashset_scan. */ +static __attribute__((noinline, cold)) bool +hashset_adopt_probe(hashset_t* hs, int8_t probe_type) { + if (!hs->num_f64 && hs_needs_num_f64(hs->src_type, probe_type)) { + hs->num_f64 = true; + /* Only int cells change hash in this mode: a float- or list-built + * set already sits in the f64 layout, so it needs no rehash. */ + if (hs_num_class(hs->src_type) == HS_NUM_INT && + !hashset_rehash(hs, hs->cap)) { + hs->num_f64 = false; + return false; + } + } + hs->probe_type = probe_type; + return true; +} + +/* Hash-free probe for when the table could not be rehashed: compare the + * row against every stored one. */ +static __attribute__((noinline, cold)) int64_t +hashset_scan(hashset_t* hs, ray_t* probe_src, int64_t probe_i, + int8_t probe_type, void* probe_data) { + for (int64_t k = 0; k < hs->cap; k++) { + int64_t stored = hs->slots[k]; + if (stored != HS_EMPTY && + hs_eq_rows(probe_src, probe_i, probe_type, probe_data, + hs->src, stored, hs->src_type, hs->src_data)) + return stored; + } + return HS_EMPTY; +} + static int64_t hashset_find_xrow(hashset_t* hs, ray_t* probe_src, int64_t probe_i, int8_t probe_type, void* probe_data) { if (hs_row_is_null(probe_src, probe_i, probe_data)) @@ -277,7 +373,11 @@ static int64_t hashset_find_xrow(hashset_t* hs, ray_t* probe_src, int64_t probe_ return HS_EMPTY; } } - uint64_t h = hs_hash_row(probe_src, probe_i, probe_type, probe_data); + if (probe_type != hs->probe_type && + !hashset_adopt_probe(hs, probe_type)) + return hashset_scan(hs, probe_src, probe_i, probe_type, probe_data); + uint64_t h = hs_hash_row(probe_src, probe_i, probe_type, probe_data, + hs->num_f64); int64_t s = (int64_t)(h & (uint64_t)hs->mask); while (hs->slots[s] != HS_EMPTY) { int64_t stored = hs->slots[s]; @@ -366,7 +466,8 @@ static bool hashset_insert(hashset_t* hs, int64_t i) { if (hs->count * 2 >= hs->cap) { if (!hashset_grow(hs)) { /* fall through, may degrade */ } } - uint64_t h = hs_hash_row(hs->src, i, hs->src_type, hs->src_data); + uint64_t h = hs_hash_row(hs->src, i, hs->src_type, hs->src_data, + hs->num_f64); int64_t s = (int64_t)(h & (uint64_t)hs->mask); while (hs->slots[s] != HS_EMPTY) { int64_t stored = hs->slots[s]; diff --git a/test/rfl/collection/collection_branch_cov.rfl b/test/rfl/collection/collection_branch_cov.rfl index dc0e28bb8..24708f789 100644 --- a/test/rfl/collection/collection_branch_cov.rfl +++ b/test/rfl/collection/collection_branch_cov.rfl @@ -218,12 +218,11 @@ ;; STR equality (lines 145-151) (count (union ["aa" "bb"] ["bb" "cc"])) -- 3 -;; Cross-type comparison via atom_eq fallback (lines 159-165) -;; except where vec1 is I64 and vec2 is F64 → cross-type hs_eq_rows -;; NOTE: hash values differ across types (I64 vs F64), so the hashset -;; probe may miss. The typed-vec path uses type-specific hashing, so -;; cross-type except returns all of vec1 (no matches found). -(count (except [1 2 3 4] (as 'F64 [2 3]))) -- 4 +;; Cross-type comparison: except where vec1 is I64 and vec2 is F64 → +;; cross-type hs_eq_rows. The first I64 probe switches the F64-built set +;; into num_f64 mode so 2 and 2.0 share a bucket; this used to return all +;; of vec1 because the two sides hashed through different functions (#645). +(count (except [1 2 3 4] (as 'F64 [2 3]))) -- 2 ;; ══════════════════════════════════════════════════════════════════════ ;; Section 10: hashset_grow — trigger by exceeding load factor (lines 202-226) diff --git a/test/rfl/collection/mixed_numeric.rfl b/test/rfl/collection/mixed_numeric.rfl new file mode 100644 index 000000000..028d45da9 --- /dev/null +++ b/test/rfl/collection/mixed_numeric.rfl @@ -0,0 +1,59 @@ +;; Set operations over mixed int/float typed vectors (#645). +;; +;; Numeric equality crosses widths and int/float: (== 3 3.0) is true, and +;; atom_eq compares any two numerics through f64. The row hashset used by +;; find/except/union/sect/in hashed int cells with ray_hash_i64 and float +;; cells with ray_hash_f64, so equal values landed in different buckets and +;; the probe missed; a correct answer was a hash collision. The set now +;; switches to hashing int cells through f64 when it meets a float probe. + +;; the contract these pin +(== [1 2 3] [1.0 2.0 3.0]) -- [true true true] + +;; find — both directions, and a haystack larger than the needles +(find [1.0 2.0 3.0] [3 1]) -- [2 0] +(find [1 2 3] [3.0 1.0]) -- [2 0] +(find (as 'F64 (til 100)) [3 1]) -- [3 1] +(find (til 100) [3.0 1.0 2.5]) -- [3 1 0Nl] + +;; except +(except [1 2 3] [1.0 3.0]) -- [2] +(except [1.0 2.0 3.0] [1 3]) -- [2.0] +(except [1i 2i 3i] [2.0]) -- [1i 3i] +(except [1h 2h] [2.0]) -- [1h] + +;; union — the shared value appears once +(count (union [1 2] [2.0 3.0])) -- 3 + +;; sect +(sect [1 2 3] [2.0 3.0]) -- [2 3] +(sect [1.0 2.0 3.0] [3 9]) -- [3.0] + +;; in with a null on BOTH sides still reaches the hashset +(in [1 0Nl 3] [1.0 0Nf]) -- [true true false] +(in [1.0 0Nf 3.0] [0Nl 3]) -- [false true true] + +;; a fractional float matches no int +(find [1 2 3] [2.5]) -- [0Nl] +(except [1 2 3] [2.5]) -- [1 2 3] + +;; same-type sets are unaffected +(find [1 2 3] [3 1]) -- [2 0] +(except [1 2 3] [1 3]) -- [2] +(find [1.0 2.0] [2.0]) -- [1] + +;; a float-built set probed by ints (no rehash needed), then by floats +(set _fs (as 'F64 (til 1000))) +(count (except (til 2000) _fs)) -- 1000 +(count (except (as 'F64 (til 2000)) _fs)) -- 1000 + +;; beyond 2^53 an int and a double compare through f64, so an int matches +;; the double its neighbour rounds to — (== 9007199254740993 +;; 9007199254740992.0) is true too. Same-type ints that share a double +;; image are still told apart. +(find [9007199254740993 9007199254740992 9007199254740994] [9007199254740992.0 9007199254740994.0]) -- [0 2] +(except [9007199254740993 9007199254740992 9007199254740994] [9007199254740992]) -- [9007199254740993 9007199254740994] + +;; enough rows to force hashset growth before and after the switch +(count (except (til 1000) (as 'F64 (til 500)))) -- 500 +(sum (find (as 'F64 (til 1000)) (til 1000))) -- 499500