diff --git a/src/ops/query.c b/src/ops/query.c index 34525440..a03cf8ce 100644 --- a/src/ops/query.c +++ b/src/ops/query.c @@ -9475,9 +9475,12 @@ ray_t* ray_update(ray_t** args, int64_t n) { } ray_release(groups); - /* Store in-place if needed */ - if (inplace_sym >= 0) { + /* Store in-place and return the symbol if amending by name. */ + if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { ray_env_set(inplace_sym, result); + ray_release(result); + ray_release(tbl); + return ray_sym(inplace_sym); } ray_release(tbl); return result; @@ -9825,6 +9828,9 @@ ray_t* ray_update(ray_t** args, int64_t n) { ray_release(mask_vec); if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { ray_env_set(inplace_sym, result); + ray_release(result); + ray_release(tbl); + return ray_sym(inplace_sym); } ray_release(tbl); return result; @@ -10115,9 +10121,12 @@ ray_t* ray_update(ray_t** args, int64_t n) { if (RAY_IS_ERR(result)) { ray_release(tbl); return result; } } - /* Store in-place if from: 't */ + /* Store in-place and return the symbol if amending by name (from: 't). */ if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { ray_env_set(inplace_sym, result); + ray_release(result); + ray_release(tbl); + return ray_sym(inplace_sym); } ray_release(tbl); return result; @@ -10131,7 +10140,11 @@ ray_t* ray_insert_fn(ray_t** args, int64_t n) { ray_t* ray_insert(ray_t** args, int64_t n) { if (n < 2) return ray_error("domain", NULL); - /* Special form: detect 'sym (quoted symbol for in-place insert) */ + /* In-place vs functional is decided by the EVALUATED first argument: + * a symbol value names a global to amend in place (returning that + * symbol); a table/vec value is amended functionally (returning the + * new value). Evaluating first means it works however the symbol + * arrives — literal 'sym, (quote sym), a variable, or a lambda param. */ int64_t inplace_sym = -1; ray_t* tbl_raw = args[0]; ray_t* tbl; @@ -10139,18 +10152,20 @@ ray_t* ray_insert(ray_t** args, int64_t n) { /* Detect calling convention: already-evaluated args (from upsert) vs raw parse tree */ int already_eval = (tbl_raw && tbl_raw->type == RAY_TABLE); - if (!already_eval && tbl_raw && tbl_raw->type == -RAY_SYM && (tbl_raw->attrs & ATTR_QUOTED)) { - /* Quoted/literal symbol 'sym (ATTR_QUOTED set) — in-place insert */ - inplace_sym = tbl_raw->i64; - tbl = ray_env_get(inplace_sym); - if (!tbl || RAY_IS_ERR(tbl)) return ray_error("domain", NULL); - ray_retain(tbl); - } else if (already_eval) { + if (already_eval) { tbl = tbl_raw; ray_retain(tbl); } else { tbl = ray_eval(tbl_raw); if (!tbl || RAY_IS_ERR(tbl)) return tbl ? tbl : ray_error("type", NULL); + if (tbl->type == -RAY_SYM) { + /* Symbol value → resolve the named global for in-place insert */ + inplace_sym = tbl->i64; + ray_release(tbl); + tbl = ray_env_get(inplace_sym); + if (!tbl || RAY_IS_ERR(tbl)) return ray_error("domain", NULL); + ray_retain(tbl); + } } /* ==================================================================== @@ -10313,8 +10328,10 @@ ray_t* ray_insert(ray_t** args, int64_t n) { } if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { + /* In-place amend stores the new value and returns the symbol. */ ray_env_set(inplace_sym, result); - ray_retain(result); + ray_release(result); + return ray_sym(inplace_sym); } return result; } @@ -10538,11 +10555,11 @@ ray_t* ray_insert(ray_t** args, int64_t n) { ray_release(tbl); ray_release(row_orig); - /* In-place: update the variable in the env */ - if (inplace_sym >= 0 && !RAY_IS_ERR(result)) { + /* In-place: store the new table in the env and return the symbol. */ + if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { ray_env_set(inplace_sym, result); - ray_retain(result); - return result; + ray_release(result); + return ray_sym(inplace_sym); } return result; } @@ -10562,17 +10579,20 @@ ray_t* ray_upsert(ray_t** args, int64_t n) { int already_eval = (tbl_raw && tbl_raw->type == RAY_TABLE); ray_t* tbl; - if (!already_eval && tbl_raw && tbl_raw->type == -RAY_SYM && (tbl_raw->attrs & ATTR_QUOTED)) { - inplace_sym = tbl_raw->i64; - tbl = ray_env_get(inplace_sym); - if (!tbl || RAY_IS_ERR(tbl)) return ray_error("domain", NULL); - ray_retain(tbl); - } else if (already_eval) { + if (already_eval) { tbl = tbl_raw; ray_retain(tbl); } else { tbl = ray_eval(tbl_raw); if (!tbl || RAY_IS_ERR(tbl)) return tbl ? tbl : ray_error("type", NULL); + if (tbl->type == -RAY_SYM) { + /* Symbol value → resolve the named global for in-place upsert */ + inplace_sym = tbl->i64; + ray_release(tbl); + tbl = ray_env_get(inplace_sym); + if (!tbl || RAY_IS_ERR(tbl)) return ray_error("domain", NULL); + ray_retain(tbl); + } } ray_t* key_sym = already_eval ? (ray_retain(args[1]), args[1]) : ray_eval(args[1]); @@ -10687,9 +10707,10 @@ ray_t* ray_upsert(ray_t** args, int64_t n) { ray_release(tbl); ray_release(key_sym); ray_release(row); - if (inplace_sym >= 0 && !RAY_IS_ERR(cur_tbl)) { + if (inplace_sym >= 0 && cur_tbl && !RAY_IS_ERR(cur_tbl)) { ray_env_set(inplace_sym, cur_tbl); - ray_retain(cur_tbl); + ray_release(cur_tbl); + return ray_sym(inplace_sym); } return cur_tbl; } @@ -10811,9 +10832,10 @@ ray_t* ray_upsert(ray_t** args, int64_t n) { ray_release(tbl); ray_release(key_sym); ray_release(row); - if (inplace_sym >= 0 && !RAY_IS_ERR(cur_tbl)) { + if (inplace_sym >= 0 && cur_tbl && !RAY_IS_ERR(cur_tbl)) { ray_env_set(inplace_sym, cur_tbl); - ray_retain(cur_tbl); + ray_release(cur_tbl); + return ray_sym(inplace_sym); } return cur_tbl; } @@ -10893,9 +10915,10 @@ ray_t* ray_upsert(ray_t** args, int64_t n) { ray_release(tbl); ray_release(key_sym); ray_release(row); - if (inplace_sym >= 0 && !RAY_IS_ERR(result)) { + if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { ray_env_set(inplace_sym, result); - ray_retain(result); + ray_release(result); + return ray_sym(inplace_sym); } return result; } @@ -10973,9 +10996,10 @@ ray_t* ray_upsert(ray_t** args, int64_t n) { ray_release(key_sym); ray_release(row); - if (inplace_sym >= 0 && !RAY_IS_ERR(result)) { + if (inplace_sym >= 0 && result && !RAY_IS_ERR(result)) { ray_env_set(inplace_sym, result); - ray_retain(result); + ray_release(result); + return ray_sym(inplace_sym); } return result; } diff --git a/src/ops/tblop.c b/src/ops/tblop.c index b9b47512..4b5c33eb 100644 --- a/src/ops/tblop.c +++ b/src/ops/tblop.c @@ -686,9 +686,9 @@ ray_t* ray_alter_fn(ray_t** args, int64_t n) { } ray_release(idx); ray_release(val); ray_env_set(name_sym->i64, new_list); - ray_release(name_sym); - ray_retain(new_list); - return new_list; + ray_release(new_list); + /* In-place amend returns the symbol, not the altered value. */ + return name_sym; } /* `var` came from ray_env_get as a BORROWED ref. ray_cow's @@ -781,12 +781,11 @@ ray_t* ray_alter_fn(ray_t** args, int64_t n) { } ray_release(val); ray_env_set(name_sym->i64, var); - ray_release(name_sym); /* The retain-first at the top of the set path gave us an owning - * ref to var. ray_env_set already retained for the env binding; - * transferring our existing ref to the caller via return is - * correct. No additional ray_retain here. */ - return var; + * ref to var; ray_env_set retained its own for the binding, so + * drop ours. In-place amend returns the symbol, not the value. */ + ray_release(var); + return name_sym; } if (olen == 6 && memcmp(oname, "concat", 6) == 0) { /* (alter 'v concat val) */ @@ -798,9 +797,9 @@ ray_t* ray_alter_fn(ray_t** args, int64_t n) { ray_release(val); if (RAY_IS_ERR(new_vec)) { ray_release(name_sym); return new_vec; } ray_env_set(name_sym->i64, new_vec); - ray_release(name_sym); - ray_retain(new_vec); - return new_vec; + ray_release(new_vec); + /* In-place amend returns the symbol, not the altered value. */ + return name_sym; } if (olen == 6 && memcmp(oname, "remove", 6) == 0) { /* (alter 'v remove idx) — remove element(s) at index/indices */ @@ -860,9 +859,9 @@ ray_t* ray_alter_fn(ray_t** args, int64_t n) { } new_list->len = j; ray_env_set(name_sym->i64, new_list); - ray_release(name_sym); - ray_retain(new_list); - return new_list; + ray_release(new_list); + /* In-place amend returns the symbol, not the altered value. */ + return name_sym; } ray_release(op_name); ray_release(name_sym); diff --git a/test/rfl/linkop/coverage.rfl b/test/rfl/linkop/coverage.rfl index b6e135a5..eb98cf9c 100644 --- a/test/rfl/linkop/coverage.rfl +++ b/test/rfl/linkop/coverage.rfl @@ -77,7 +77,7 @@ linked_i32.age -- [42 18 25 42] ;; I32 link column with a null link row (null row propagates to result) (set rids_i32n (as 'I32 [0 1 2 0])) -(set rids_i32n (alter 'rids_i32n set 1 0Ni)) +(alter 'rids_i32n set 1 0Ni) (set linked_i32n (.col.link 'dim_i32 rids_i32n)) (set res_i32n linked_i32n.age) (nil? (at res_i32n 1)) -- true @@ -120,7 +120,7 @@ linked_i32.age -- [42 18 25 42] (set dim_f64 (table [id score] (list [100 200 300] (as 'F64 [1.5 2.5 3.5])))) (set rids_f64 [0 1 2 0]) -(set rids_f64n (alter 'rids_f64 set 1 0Nl)) +(alter 'rids_f64 set 1 0Nl)(set rids_f64n rids_f64) (set linked_f64 (.col.link 'dim_f64 rids_f64n)) (set res_f64 linked_f64.score) (nil? (at res_f64 1)) -- true @@ -135,7 +135,7 @@ linked_i32.age -- [42 18 25 42] (set dim_i32t (table [id score32] (list [100 200 300] (as 'I32 [10 20 30])))) (set rids_i32t [0 1 2 0]) -(set rids_i32tn (alter 'rids_i32t set 1 0Nl)) +(alter 'rids_i32t set 1 0Nl)(set rids_i32tn rids_i32t) (set linked_i32t (.col.link 'dim_i32t rids_i32tn)) (set res_i32t linked_i32t.score32) (nil? (at res_i32t 1)) -- true @@ -149,7 +149,7 @@ linked_i32.age -- [42 18 25 42] (set dim_dt (table [id dt] (list [100 200 300] [2024.01.01 2024.06.15 2024.12.31]))) (set rids_dt [0 1 2 0]) -(set rids_dtn (alter 'rids_dt set 2 0Nl)) +(alter 'rids_dt set 2 0Nl)(set rids_dtn rids_dt) (set linked_dt (.col.link 'dim_dt rids_dtn)) (set res_dt linked_dt.dt) (nil? (at res_dt 2)) -- true @@ -163,7 +163,7 @@ linked_i32.age -- [42 18 25 42] (set dim_tm (table [id tm] (list [100 200 300] (as 'TIME [3600000000000 7200000000000 10800000000000])))) (set rids_tm [0 1 2 1]) -(set rids_tmn (alter 'rids_tm set 0 0Nl)) +(alter 'rids_tm set 0 0Nl)(set rids_tmn rids_tm) (set linked_tm (.col.link 'dim_tm rids_tmn)) (set res_tm linked_tm.tm) ;; row 0 is null link -> null TIME @@ -176,7 +176,7 @@ linked_i32.age -- [42 18 25 42] (set dim_i16 (table [id val16] (list [100 200 300] (as 'I16 [10 20 30])))) (set rids_i16 [0 1 2 0]) -(set rids_i16n (alter 'rids_i16 set 1 0Nl)) +(alter 'rids_i16 set 1 0Nl)(set rids_i16n rids_i16) (set linked_i16 (.col.link 'dim_i16 rids_i16n)) (set res_i16 linked_i16.val16) (nil? (at res_i16 1)) -- true @@ -199,7 +199,7 @@ linked_sd.name -- (list 'carol 'alice 'bob 'alice) ;; sym_dict propagation: null in link -> null SYM in result (set rids_sdn [2 0 1 0]) -(set rids_sdn (alter 'rids_sdn set 1 0Nl)) +(alter 'rids_sdn set 1 0Nl) (set linked_sdn (.col.link 'dim_sd rids_sdn)) (set res_sdn linked_sdn.name) (.col.link? linked_sdn) -- true diff --git a/test/rfl/ops/query_coverage.rfl b/test/rfl/ops/query_coverage.rfl index 2092b2c7..c46d20aa 100644 --- a/test/rfl/ops/query_coverage.rfl +++ b/test/rfl/ops/query_coverage.rfl @@ -319,7 +319,7 @@ ;; (mask=true → expr_vec, mask=false → orig_col) hit the ;; ray_read_sym dispatch. (set TupL (table [id name] (list [1 2 3] ['alice 'bob 'charlie]))) -(set TupL2 (update {name: 'XXX from: 'TupL where: (== id 2)})) +(set TupL2 (update {name: 'XXX from: TupL where: (== id 2)})) (at (at TupL2 'name) 0) -- 'alice (at (at TupL2 'name) 1) -- 'XXX (at (at TupL2 'name) 2) -- 'charlie @@ -343,12 +343,12 @@ ;; ==================================================================== (set TupS (table [id s] (list [1 2 3 4] (list "a" "b" "c" "d")))) -(set TupS2 (update {s: "XX" from: 'TupS where: (> id 2)})) +(set TupS2 (update {s: "XX" from: TupS where: (> id 2)})) (first (at TupS2 's)) -- "a" (at (at TupS2 's) 2) -- "XX" (set TupSy (table [id k] (list [1 2 3] ['a 'b 'c]))) -(set TupSy2 (update {k: 'XX from: 'TupSy where: (== id 2)})) +(set TupSy2 (update {k: 'XX from: TupSy where: (== id 2)})) (at (at TupSy2 'k) 0) -- 'a (at (at TupSy2 'k) 1) -- 'XX diff --git a/test/rfl/table/insert_inplace_symbol.rfl b/test/rfl/table/insert_inplace_symbol.rfl new file mode 100644 index 00000000..c50c075d --- /dev/null +++ b/test/rfl/table/insert_inplace_symbol.rfl @@ -0,0 +1,71 @@ +;; insert_inplace_symbol.rfl — in-place amend via a symbol VALUE, not just a +;; literal 'sym. The amend's first argument is evaluated; if it yields a +;; symbol, the amend targets the named global in place and returns that +;; symbol (matching v1 / the reference dialect). If it yields a table/vec, +;; the amend is functional: it returns a fresh value and leaves the +;; variable untouched. +;; +;; Regression: (insert name row) with name holding 't raised `type` because +;; only a literal 't (ATTR_QUOTED on the raw parse node) was recognised — a +;; symbol produced by evaluation slipped through to the vec/list path. + +;; ── insert: a variable holding a symbol drives in-place append ──────── +(set t (table [a] (list []))) +(set name 't) +(insert name (list 1)) +(at t 'a) -- [1] +;; the reported bug: a second insert through the same variable +(insert name (list 2)) +(at t 'a) -- [1 2] + +;; in-place insert returns the SYMBOL, not the amended table +(insert name (list 3)) -- 't +(insert 't (list 4)) -- 't +(at t 'a) -- [1 2 3 4] + +;; (quote t) is equivalent to 't and also drives in-place insert +(insert (quote t) (list 5)) +(at t 'a) -- [1 2 3 4 5] + +;; functional form (value first arg) returns the table and does NOT mutate +;; the bound variable +(set u (table [a] (list [10]))) +(insert u (list 20)) -- (table [a] (list [10 20])) +(at u 'a) -- [10] + +;; ── upsert: a variable holding a symbol ────────────────────────────── +(set kt (table [id v] (list [1 2] [10 20]))) +(set kn 'kt) +(upsert kn 1 (list 3 30)) -- 'kt +(at kt 'id) -- [1 2 3] +(upsert kn 1 (list 2 99)) -- 'kt +(at (at kt 'v) 1) -- 99 + +;; ── update: from-table named through a variable ────────────────────── +(set ut (table [id v] (list [1 2 3] [10 20 30]))) +(set un 'ut) +(update {from: un v: 99}) -- 'ut +(at ut 'v) -- [99 99 99] +;; literal 'sym update still returns the symbol +(update {from: 'ut v: 7}) -- 'ut +(at ut 'v) -- [7 7 7] + +;; ── works the same way inside a lambda ─────────────────────────────── +(set lt (table [a] (list []))) +((fn [s r] (insert s r)) 'lt (list 7)) +(at lt 'a) -- [7] +;; symbol passed through a lambda PARAM, in-place, returns the symbol +((fn [s r] (insert s r)) 'lt (list 8)) -- 'lt +(at lt 'a) -- [7 8] + +;; ── alter (in-place vector/list amend) also returns the symbol ──────── +(set av [1 2 3]) +(alter 'av concat 4) -- 'av +(at av 3) -- 4 +(alter 'av set 0 99) -- 'av +(at av 0) -- 99 +;; alter through a variable holding the symbol, and through a lambda param +(set an 'av) +(alter an concat 5) -- 'av +((fn [s] (alter s concat 6)) 'av) -- 'av +(count av) -- 6 diff --git a/test/rfl/table/update.rfl b/test/rfl/table/update.rfl index d8264c6d..678edbfa 100644 --- a/test/rfl/table/update.rfl +++ b/test/rfl/table/update.rfl @@ -193,13 +193,14 @@ t -- (table [ID Name Value] (list [1 2] [alice bob] [10.0 20.0])) ;; ══════════════════════════════════════════════════════════════════ ;; Narrow-int and temporal columns keep their type after a scalar update. -(set t (table [k a] (list [1 2 3] (as 'I32 [10 20 30]))))(at (update {a: 99i from: 't}) 'a) -- [99i 99i 99i] -(set t (table [k a] (list [1 2 3] (as 'I16 [1 2 3]))))(at (update {a: 7h from: 't}) 'a) -- [7h 7h 7h] -(set t (table [k a] (list [1 2 3] (as 'Date [2020.01.01 2020.01.02 2020.01.03]))))(at (update {a: 2030.06.15 from: 't}) 'a) -- [2030.06.15 2030.06.15 2030.06.15] -(set t (table [k a] (list [1 2 3] (as 'Timestamp [2024.01.01D00:00:00.0 2024.01.02D00:00:00.0 2024.01.03D00:00:00.0]))))(at (update {a: 2030.01.01D00:00:00.0 from: 't}) 'a) -- [2030.01.01D00:00:00.000000000 2030.01.01D00:00:00.000000000 2030.01.01D00:00:00.000000000] +;; In-place update (from: 't) returns the symbol, so read the column back off t. +(set t (table [k a] (list [1 2 3] (as 'I32 [10 20 30]))))(update {a: 99i from: 't})(at t 'a) -- [99i 99i 99i] +(set t (table [k a] (list [1 2 3] (as 'I16 [1 2 3]))))(update {a: 7h from: 't})(at t 'a) -- [7h 7h 7h] +(set t (table [k a] (list [1 2 3] (as 'Date [2020.01.01 2020.01.02 2020.01.03]))))(update {a: 2030.06.15 from: 't})(at t 'a) -- [2030.06.15 2030.06.15 2030.06.15] +(set t (table [k a] (list [1 2 3] (as 'Timestamp [2024.01.01D00:00:00.0 2024.01.02D00:00:00.0 2024.01.03D00:00:00.0]))))(update {a: 2030.01.01D00:00:00.0 from: 't})(at t 'a) -- [2030.01.01D00:00:00.000000000 2030.01.01D00:00:00.000000000 2030.01.01D00:00:00.000000000] ;; GUID column scalar update (all-rows): every row becomes the scalar guid. ;; (guid is random, so compare cells to the scalar rather than a literal.) -(set g (first (guid 1)))(set t (table [k a] (list [1 2 3] (guid 3))))(== (at (at (update {a: g from: 't}) 'a) 2) g) -- true +(set g (first (guid 1)))(set t (table [k a] (list [1 2 3] (guid 3))))(update {a: g from: 't})(== (at (at t 'a) 2) g) -- true ;; GUID column scalar update (WHERE): only matching rows change, no overflow. -(set g (first (guid 1)))(set t (table [k a] (list [1 2 3] (guid 3))))(== (at (at (update {a: g from: 't where: (== k 2)}) 'a) 1) g) -- true +(set g (first (guid 1)))(set t (table [k a] (list [1 2 3] (guid 3))))(update {a: g from: 't where: (== k 2)})(== (at (at t 'a) 1) g) -- true diff --git a/test/test_lang.c b/test/test_lang.c index 24a8c364..ce9776c1 100644 --- a/test/test_lang.c +++ b/test/test_lang.c @@ -2408,8 +2408,9 @@ static test_result_t test_eval_insert(void) { static test_result_t test_eval_insert_vec_append(void) { ASSERT_EQ("(do (set v (til 5)) (insert 'v 99) v)", "[0 1 2 3 4 99]"); ASSERT_EQ("(do (set v (til 3)) (insert 'v [10 20 30]) v)", "[0 1 2 10 20 30]"); - /* Return value mirrors the rebound v */ - ASSERT_EQ("(do (set v (til 3)) (insert 'v 9))", "[0 1 2 9]"); + /* In-place amend (symbol target) returns the symbol, not the rebound vec */ + ASSERT_EQ("(do (set v (til 3)) (insert 'v 9))", "'v"); + ASSERT_EQ("(do (set v (til 3)) (insert 'v 9) v)", "[0 1 2 9]"); PASS(); }