From 86aceccc511f78079af54df9cb6d6da488849afd Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 12 Aug 2026 17:41:50 +0200 Subject: [PATCH 1/2] fix(store): repair legacy string hash caches --- src/store/col.c | 20 +++++++++-- test/test_store.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/store/col.c b/src/store/col.c index 7f0c17db..d36855d5 100644 --- a/src/store/col.c +++ b/src/store/col.c @@ -1212,7 +1212,8 @@ static ray_err_t col_validate_str_region(ray_t* hdr, const void* ptr, if (pool_size > mapped_size - offset - 32) return RAY_ERR_CORRUPT; - const ray_str_t* elems = (const ray_str_t*)((const char*)ptr + 32); + ray_str_t* elems = (ray_str_t*)((char*)ptr + 32); + const char* pool_base = (const char*)ptr + offset + 32; for (int64_t i = 0; i < hdr->len; i++) { uint32_t len = elems[i].len; if (len <= RAY_STR_INLINE_MAX) continue; @@ -1220,12 +1221,27 @@ static ray_err_t col_validate_str_region(ray_t* hdr, const void* ptr, len > pool_size - elems[i].pool_off) return RAY_ERR_CORRUPT; if (len >= 4) { - const char* p = (const char*)ptr + offset + 32 + elems[i].pool_off; + const char* p = pool_base + elems[i].pool_off; if (memcmp(elems[i].prefix, p, 4) != 0) return RAY_ERR_CORRUPT; } } + /* Pre-hash-cache column files persisted the descriptor's final four + * bytes as uninitialized padding. A nonzero legacy value must never be + * trusted as a content hash: equal strings could otherwise probe + * different join/group slots and silently produce wrong results. + * + * ray_vm_map_file is MAP_PRIVATE / copy-on-write, so refreshing the + * validated descriptors repairs both callers: ray_col_load copies these + * values into its buddy block, while ray_col_mmap retains the private + * repaired mapping without modifying the file. */ + for (int64_t i = 0; i < hdr->len; i++) { + if (ray_str_is_inline(&elems[i])) continue; + uint32_t h = (uint32_t)ray_str_t_hash(&elems[i], pool_base); + elems[i].hash32 = h != 0 ? h : 1u; + } + out->has_str_pool = true; out->str_pool_offset = offset; out->str_pool_size = pool_size; diff --git a/test/test_store.c b/test/test_store.c index 6001e6b5..03e37ad1 100644 --- a/test/test_store.c +++ b/test/test_store.c @@ -41,8 +41,10 @@ #include "core/platform.h" #include "core/runtime.h" #include "lang/eval.h" +#include "lang/internal.h" #include "mem/sys.h" #include "table/sym.h" +#include "vec/str.h" #ifndef RAY_OS_WINDOWS #include @@ -4363,6 +4365,87 @@ static test_result_t test_col_str_pool_roundtrip(void) { PASS(); } +/* ---- test_col_str_legacy_hash_repair ----------------------------------- */ +/* Older column writers persisted the pooled descriptor's final four bytes + * as uninitialized padding. Forge distinct nonzero values there for equal + * strings and verify both load modes recompute content hashes before the + * grouping fast path consumes them. */ +static test_result_t test_col_str_legacy_hash_repair(void) { + const char* same = "legacy-pooled-string-value"; + const char* other = "different-pooled-string"; + ray_t* vec = ray_vec_new(RAY_STR, 3); + TEST_ASSERT_FALSE(RAY_IS_ERR(vec)); + vec = ray_str_vec_append(vec, same, strlen(same)); + TEST_ASSERT_FALSE(RAY_IS_ERR(vec)); + vec = ray_str_vec_append(vec, same, strlen(same)); + TEST_ASSERT_FALSE(RAY_IS_ERR(vec)); + vec = ray_str_vec_append(vec, other, strlen(other)); + TEST_ASSERT_FALSE(RAY_IS_ERR(vec)); + TEST_ASSERT_EQ_I(ray_col_save(vec, TMP_COL_PATH), RAY_OK); + + /* [32-byte column header][16-byte descriptors]; hash32 is descriptor + * bytes 12..15. These model arbitrary nonzero legacy padding. */ + static const uint32_t garbage[] = { + 0x11111111u, 0x22222222u, 0x33333333u + }; + FILE* f = fopen(TMP_COL_PATH, "r+b"); + TEST_ASSERT_NOT_NULL(f); + for (int i = 0; i < 3; i++) { + TEST_ASSERT_EQ_I(fseek(f, 32L + (long)i * 16L + 12L, SEEK_SET), 0); + TEST_ASSERT_EQ_U(fwrite(&garbage[i], 1, sizeof(garbage[i]), f), + sizeof(garbage[i])); + } + TEST_ASSERT_EQ_I(fclose(f), 0); + + ray_t* loaded = ray_col_load(TMP_COL_PATH); + TEST_ASSERT_NOT_NULL(loaded); + TEST_ASSERT_FALSE(RAY_IS_ERR(loaded)); + ray_str_t* ld = (ray_str_t*)ray_data(loaded); + const char* lpool = (const char*)ray_data(loaded->str_pool); + for (int i = 0; i < 3; i++) { + uint32_t h = (uint32_t)ray_str_t_hash(&ld[i], lpool); + TEST_ASSERT_EQ_U(ld[i].hash32, h != 0 ? h : 1u); + } + TEST_ASSERT_EQ_U(ld[0].hash32, ld[1].hash32); + ray_t* groups = ray_group_indices_fn(loaded); + TEST_ASSERT_NOT_NULL(groups); + TEST_ASSERT_FALSE(RAY_IS_ERR(groups)); + TEST_ASSERT_EQ_I(ray_dict_keys(groups)->len, 2); + ray_release(groups); + ray_release(loaded); + + ray_t* mapped = ray_col_mmap(TMP_COL_PATH); + TEST_ASSERT_NOT_NULL(mapped); + TEST_ASSERT_FALSE(RAY_IS_ERR(mapped)); + ray_str_t* md = (ray_str_t*)ray_data(mapped); + const char* mpool = (const char*)ray_data(mapped->str_pool); + for (int i = 0; i < 3; i++) { + uint32_t h = (uint32_t)ray_str_t_hash(&md[i], mpool); + TEST_ASSERT_EQ_U(md[i].hash32, h != 0 ? h : 1u); + } + TEST_ASSERT_EQ_U(md[0].hash32, md[1].hash32); + groups = ray_group_indices_fn(mapped); + TEST_ASSERT_NOT_NULL(groups); + TEST_ASSERT_FALSE(RAY_IS_ERR(groups)); + TEST_ASSERT_EQ_I(ray_dict_keys(groups)->len, 2); + ray_release(groups); + ray_release(mapped); + + /* MAP_PRIVATE repair must not rewrite the persisted legacy bytes. */ + f = fopen(TMP_COL_PATH, "rb"); + TEST_ASSERT_NOT_NULL(f); + TEST_ASSERT_EQ_I(fseek(f, 32L + 12L, SEEK_SET), 0); + uint32_t persisted = 0; + TEST_ASSERT_EQ_U(fread(&persisted, 1, sizeof(persisted), f), + sizeof(persisted)); + TEST_ASSERT_EQ_I(fclose(f), 0); + TEST_ASSERT_EQ_U(persisted, garbage[0]); + + ray_release(vec); + unlink(TMP_COL_PATH); + PASS(); +} + /* ---- test_col_format_version_roundtrip ---------------------------------- */ /* A saved column carries the format generation in the 32-byte header's * `order` byte (offset 17), with aux (bytes 0-15) ZERO on disk (no magic — @@ -5103,6 +5186,7 @@ const test_entry_t store_entries[] = { { "store/col_recursive_sym_in_list", test_col_recursive_sym_in_list, store_setup, store_teardown }, { "store/col_sym_w64_neg_index", test_col_sym_w64_negative_index, store_setup, store_teardown }, { "store/col_str_pool_roundtrip", test_col_str_pool_roundtrip, store_setup, store_teardown }, + { "store/col_str_legacy_hash_repair", test_col_str_legacy_hash_repair, store_setup, store_teardown }, { "store/col_format_version_roundtrip", test_col_format_version_roundtrip, store_setup, store_teardown }, { "store/col_format_bad_version", test_col_format_bad_version, store_setup, store_teardown }, { "store/col_str_empty_roundtrip", test_col_str_empty_roundtrip, store_setup, store_teardown }, From 48c909d7813899f1b9ee2f32b39a1bed7390bb95 Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 12 Aug 2026 17:41:58 +0200 Subject: [PATCH 2/2] fix(eval): keep structural hashing internal --- docs/docs/language/functions.md | 1 - docs/docs/reference/all-functions.md | 3 +-- src/lang/eval.c | 2 -- src/lang/eval.h | 1 - src/lang/internal.h | 1 - src/ops/builtins.c | 6 ------ test/rfl/regress/issue_394.rfl | 6 +++--- 7 files changed, 4 insertions(+), 16 deletions(-) diff --git a/docs/docs/language/functions.md b/docs/docs/language/functions.md index 0305b6ec..ef0d1aaf 100644 --- a/docs/docs/language/functions.md +++ b/docs/docs/language/functions.md @@ -160,7 +160,6 @@ Operations on vectors as collections. | `at` | binary | Index a collection; vector keys batch dictionary lookups | `(at [10 20 30] 1)` → `20` | | `find` | binary | Find index of value | `(find [10 20 30] 20)` → `1` | | `fill` | binary, atomic | Replace null values; the replacement is the first argument | `(fill 0 [1 0Nl 3])` → `[1 0 3]` | -| `hash` / `wyhash` | unary, atomic | Stable 64-bit structural hash (`wyhash` is an alias) | `(hash "abc")` | | `reverse` | unary | Reverse order | `(reverse [1 2 3])` → `[3 2 1]` | | `til` | unary | Range [0..n) | `(til 5)` → `[0 1 2 3 4]` | | `lag` | unary | Shift values one row back; first row is null/sentinel | `(lag [10 20 30])` → `[0Nl 10 20]` | diff --git a/docs/docs/reference/all-functions.md b/docs/docs/reference/all-functions.md index 8861630a..f37d04b7 100644 --- a/docs/docs/reference/all-functions.md +++ b/docs/docs/reference/all-functions.md @@ -51,7 +51,7 @@ Generated from `src/lang/eval.c` in this checkout. The categorized reference bel `.log.validate`, `rc`, `diverse`, `.time.timer.del`, `env`, `sym-name`, `dl-stratify`, `dl-eval`, `dl-free`, `norm`, `hnsw-free`, `hnsw-load`, `hnsw-info`, `.idx.zone`, `.idx.hash`, `.idx.sort`, `.idx.bloom`, `.idx.drop`, `.idx.has?`, `.idx.info`, `.attr.get`, `.attr.drop`, `.col.unlink`, `.col.link?`, `.col.target`, -`.graph.free`, `.graph.info`, `strlen`, `upper`, `lower`, `trim`, `hash`, `wyhash` +`.graph.free`, `.graph.info`, `strlen`, `upper`, `lower`, `trim` ### Binary @@ -261,7 +261,6 @@ Operations on vectors and lists as collections — set operations, indexing, sea | `at` | binary | — | Index a collection; vector keys batch dictionary lookups | `(at [10 20 30] 1)` → `20` | | `find` | binary | — | Find index of first occurrence | `(find [10 20 30] 20)` → `1` | | `fill` | binary | atomic | Replace null values; the replacement is the first argument | `(fill 0 [1 0Nl 3])` → `[1 0 3]` | -| `hash` / `wyhash` | unary | atomic | Stable 64-bit structural hash (`wyhash` is an alias) | `(hash "abc")` | | `reverse` | unary | — | Reverse element order | `(reverse [1 2 3])` → `[3 2 1]` | | `til` | unary | — | Generate range [0..n) | `(til 5)` → `[0 1 2 3 4]` | | `lag` | unary | lazy/DAG | Shift values one row back; first row is null/sentinel | `(lag [10 20 30])` → `[0Nl 10 20]` | diff --git a/src/lang/eval.c b/src/lang/eval.c index c4008252..72f8196c 100644 --- a/src/lang/eval.c +++ b/src/lang/eval.c @@ -3200,8 +3200,6 @@ static void ray_register_builtins(void) { register_binary("at", RAY_FN_NONE, ray_at_fn); register_binary("find", RAY_FN_NONE, ray_find_fn); register_binary("fill", RAY_FN_ATOMIC, ray_fill_fn); - register_unary("hash", RAY_FN_ATOMIC, ray_hash_fn); - register_unary("wyhash", RAY_FN_ATOMIC, ray_hash_fn); register_unary("reverse", RAY_FN_NONE | RAY_FN_LAZY_AWARE, ray_reverse_fn); register_unary("til", RAY_FN_NONE, ray_til_fn); register_unary_op("lag", RAY_FN_NONE | RAY_FN_LAZY_AWARE, ray_lag_fn, OP_LAG); diff --git a/src/lang/eval.h b/src/lang/eval.h index 0d333c40..d52e67b3 100644 --- a/src/lang/eval.h +++ b/src/lang/eval.h @@ -288,7 +288,6 @@ ray_t* ray_cross_fn(ray_t* a, ray_t* b); ray_t* ray_at_fn(ray_t* vec, ray_t* idx); ray_t* ray_find_fn(ray_t* vec, ray_t* val); ray_t* ray_fill_fn(ray_t* replacement, ray_t* value); -ray_t* ray_hash_fn(ray_t* x); ray_t* ray_til_fn(ray_t* x); ray_t* ray_reverse_fn(ray_t* x); ray_t* ray_lag_fn(ray_t* x); diff --git a/src/lang/internal.h b/src/lang/internal.h index 52cca04c..f5e19b6c 100644 --- a/src/lang/internal.h +++ b/src/lang/internal.h @@ -530,7 +530,6 @@ ray_t* ray_scan_left_fn(ray_t** args, int64_t n); ray_t* ray_scan_right_fn(ray_t** args, int64_t n); ray_t* ray_enlist_fn(ray_t** args, int64_t n); uint64_t ray_atom_hash(ray_t* x); -ray_t* ray_hash_fn(ray_t* x); /* String builtins (formerly static in eval.c, now in str_builtin.c) */ ray_t* ray_split_fn(ray_t* str, ray_t* delim); diff --git a/src/ops/builtins.c b/src/ops/builtins.c index 08a49acf..74c48724 100644 --- a/src/ops/builtins.c +++ b/src/ops/builtins.c @@ -2541,12 +2541,6 @@ uint64_t ray_atom_hash(ray_t* a) { } } -/* Stable 64-bit structural hash. The signed I64 result preserves every bit - * of wyhash's output; callers that display it may therefore see negatives. */ -ray_t* ray_hash_fn(ray_t* x) { - return make_i64((int64_t)ray_atom_hash(x)); -} - /* Context for GUID rehash: the 16-byte source base and, indirectly, * gvals — which stores the row_idx of the first occurrence per group. */ typedef struct { diff --git a/test/rfl/regress/issue_394.rfl b/test/rfl/regress/issue_394.rfl index bf7b7190..9c8b4a39 100644 --- a/test/rfl/regress/issue_394.rfl +++ b/test/rfl/regress/issue_394.rfl @@ -47,9 +47,9 @@ (find (list "alpha" "beta" "gamma") "beta") -- 1 (find (list "alpha" "beta" "gamma") "absent") -- 0Nl -;; Deterministic hashing is public under both descriptive and implementation names. -(== (hash 42) (wyhash 42)) -- true -(count (hash [1 2 3])) -- 3 +;; Hashing is an internal implementation detail, not a language builtin. +(hash 42) !- name +(wyhash 42) !- name ;; xkey construction is hash-based and keyed dictionaries accept vector probes. (set I394T (table [k v] (list (til 5000) (+ 10 (til 5000)))))