diff --git a/docs/pipeline/runtime.md b/docs/pipeline/runtime.md index b62221ffc..262b84d96 100644 --- a/docs/pipeline/runtime.md +++ b/docs/pipeline/runtime.md @@ -170,6 +170,25 @@ it. Snapshot tests + toolchain tests catch drift. ## Emitter ↔ runtime contract +Ruby-family lowered equality reads select SQL from the runtime value: a +non-nil value uses `col = ?` and a bind; nil uses `col IS NULL` without a slot. +The same branch selects the fragment and reserves its running bind position, +so later predicates cannot shift out of alignment. Inline emission uses +`col = ` or `col IS NULL`. `IS ?` is deliberately avoided: +SQLite excludes `IS` from its [partial-index non-null implication rule](https://www.sqlite.org/partialindex.html#queries_using_partial_indexes). + +Each nullable predicate contributes two possible fragments. Code size stays +linear: no query variants are enumerated in the compiler. A bound query with +up to seven nullable predicates has at most 128 shapes; queries above that +budget use `prepare_uncached`, avoiding exponential growth within a long +lease. Existing per-connection cache limits still apply across query sites. +Strict-target lowering retains its existing predicates and lifecycle. + +Generated Ruby-family reads use `ensure Db.finalize(stmt)` around binding, +serialization after prepare, stepping and hydration, including reload and +preloads. Text preprocessing failures also release the binder's checkout. +Cleanup therefore completes before a caller rescues within an ongoing lease. + For each target: - **Emitter assumes** specific function names, signatures, and diff --git a/runtime/ruby/active_record/connection.rb b/runtime/ruby/active_record/connection.rb index ddfac5cfe..5a15dbf3d 100644 --- a/runtime/ruby/active_record/connection.rb +++ b/runtime/ruby/active_record/connection.rb @@ -148,6 +148,20 @@ def _await_preload nil end + # Ruby accepts nil at these public entry points. Reject it before a + # key-typed adapter can coerce it to a real zero/empty-string key. + def self.find(id) + raise RecordNotFound, "Couldn't find #{name} without an ID" if id.nil? + result = _adapter_find_by_id(id) + raise RecordNotFound, "Couldn't find #{name} with id=#{id}" if result.nil? + result + end + + def self.exists?(id) + return false if id.nil? + _adapter_exists_by_id?(id) + end + # Stateless facade — every member delegates straight to `Db`, so a # fresh instance per call is cheap and dodges class-ivar state. def self.connection diff --git a/runtime/ruby/active_record/connection.rbs b/runtime/ruby/active_record/connection.rbs index f60841eff..6dab91dd2 100644 --- a/runtime/ruby/active_record/connection.rbs +++ b/runtime/ruby/active_record/connection.rbs @@ -41,6 +41,9 @@ module ActiveRecord def self.record_timestamps: () -> bool def update_column: (String | Symbol name, untyped value) -> bool def _as_json_only: (Array[Symbol] only) -> Hash[String, untyped] + # Ruby-family key guards also accept nil at public entry points. + def self.find: (Integer | String | nil id) -> Base + def self.exists?: (Integer | String | nil id) -> bool # Ruby-family overrides of base.rb's Array / COUNT fallbacks. def self.where: (Hash[Symbol, untyped] conditions) -> Relation def self.all: () -> Relation diff --git a/runtime/ruby/db.rbs b/runtime/ruby/db.rbs index 4c06788fe..da75046e4 100644 --- a/runtime/ruby/db.rbs +++ b/runtime/ruby/db.rbs @@ -14,6 +14,8 @@ module Db def self.close: () -> void def self.exec: (String) -> void def self.prepare: (String) -> Integer + # Reads with too many SQL shapes bypass the cache; finalize releases them. + def self.prepare_uncached: (String) -> Integer def self.step?: (Integer) -> bool def self.column_int: (Integer, Integer) -> Integer def self.column_float: (Integer, Integer) -> Float @@ -32,9 +34,13 @@ module Db # gate emits parameterized queries. Keys the prepared-statement cache # on the static query shape instead of per-value. Every Db shim # (cruby-gem + spinel-FFI) implements these. - def self.bind_int: (Integer, Integer, Integer) -> void + def self.bind_int: (Integer, Integer, Integer?) -> void def self.bind_text: (Integer, Integer, String) -> void - def self.bind_bool: (Integer, Integer, bool) -> void + # A nullable boolean binds SQL NULL, never the false/0 representation. + def self.bind_bool: (Integer, Integer, bool?) -> void + def self.bind_int_opt: (Integer, Integer, Integer?) -> void + def self.bind_text_opt: (Integer, Integer, String?) -> void + def self.bind_bool_opt: (Integer, Integer, bool?) -> void def self.last_insert_rowid: () -> Integer def self.changes: () -> Integer # Query-log capture (issue #27) — records the SQL each prepare/exec diff --git a/runtime/spinel/db.rb b/runtime/spinel/db.rb index ef0c2af67..b0dff658e 100644 --- a/runtime/spinel/db.rb +++ b/runtime/spinel/db.rb @@ -108,6 +108,7 @@ module SQL # coercion for the -1 destructor is the spinel primitive validated in # spinel's test/ffi_ptr_int_literal.rb. ffi_func :sqlite3_bind_int64, [:ptr, :int, :long], :int + ffi_func :sqlite3_bind_null, [:ptr, :int], :int ffi_func :sqlite3_bind_text, [:ptr, :int, :str, :int, :ptr], :int # `sqlite3_column_type` reports the storage class of a column in the # current row; 5 is SQLITE_NULL. The nullable-column reads below need @@ -341,13 +342,18 @@ def cell_text(row, i) # before the first step), and the real stmt it promoted to when the # recorded prefix ran out (0 = still replaying). class QcCursor - def initialize(entry) + def initialize(entry, preparable) @entry = entry + @preparable = preparable @pos = -1 @promoted = false @real_ptr = nil end + def preparable + @preparable + end + def promoted @promoted end @@ -498,6 +504,16 @@ def prepare_cached(sql) end i -= 1 end + prepare_owned(sql, cached) + end + + # Reads with many SQL shapes share checkout and lease cleanup with + # busy-hit transient statements. + def prepare_uncached(sql) + prepare_owned(sql, false) + end + + def prepare_owned(sql, cached) # `SQL.stmt_out` is ONE 8-byte out-buffer for the whole process (an # `ffi_buffer`, static C storage). Under parallel OS workers two # connections preparing at the same moment wrote their statement @@ -652,11 +668,11 @@ def qc_clear # The handle for `sql`: a replay handle on a hit, else the real stmt # (recording its rows as they are stepped, when the cache is on). # Returns 0 as "no replay" so `Db.prepare` can tell the two apart. - def qc_lookup(sql) + def qc_lookup(sql, preparable = true) return 0 if !@qc_on || sql.include?("?") e = @qc_by_sql[sql] return 0 if e.nil? - @qc_cursors.push(QcCursor.new(e)) + @qc_cursors.push(QcCursor.new(e, preparable)) @qc_cursors.length end @@ -766,7 +782,7 @@ def qc_step?(handle) return false if e.eof # The first consumer stopped before the end and this one wants more: # re-run the real statement and fast-forward past what was replayed. - ptr = prepare_cached(e.qc_sql) + ptr = c.preparable ? prepare_cached(e.qc_sql) : prepare_uncached(e.qc_sql) c.promote_to(ptr) n = 0 while n < e.nrows @@ -1531,6 +1547,27 @@ def self.prepare(sql) ptr end + # Bypass statement reuse for nullable shape overflow, retaining the + # separate request result cache. A partial replay must promote uncached. + # Finalize releases the transient immediately; abandoned handles close + # at lease release. Outside with_connection they remain owned until + # Db.close, so scripts must finalize explicitly after use. + def self.prepare_uncached(sql) + # Keep shared cache keys string-typed even when the compiled program + # has no transient call site from which Spinel can infer this argument. + sql = sql.to_s + conn = current_conn + handle = conn.qc_lookup(sql, false) + if handle != 0 + $stderr.puts " CACHE " + sql if @sql_trace + return handle + end + record_query(sql) + ptr = conn.prepare_uncached(sql) + conn.qc_record(sql, ptr) + ptr + end + # Query-log capture — see db_cruby.rb for the full rationale (the # test-side analog of Rails' `sql.active_record` SQLCounter; the one # instrument that can see the includes(:assoc) N+1 `compare` is blind @@ -1756,7 +1793,7 @@ def self.column_name(stmt, i) # roundhouse#12 Path A.1: with caching on, "finalize" means rewind the # cached stmt (reset cursor + clear any bound params) so the next call - # reuses it. Busy-hit transients are really finalized on release. + # reuses it. Busy-hit and explicit uncached reads are finalized on release. def self.finalize(stmt) conn = current_conn if stmt.is_a?(Integer) @@ -1777,12 +1814,41 @@ def self.finalize(stmt) # reset + clear_bindings'd at its previous `finalize`, so re-binding # here starts clean. def self.bind_int(stmt, idx, value) + # A nullable caller must not lose nil at the typed FFI integer boundary. + bind_int_opt(stmt, idx, value) + end + + def self.bind_int_opt(stmt, idx, value) + raise "Db.bind failed (21): cannot bind a replay cursor" if stmt.is_a?(Integer) + if value.nil? + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_null(stmt, idx)) + else + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_int64(stmt, idx, value)) + end + end + + def self.bind_text_opt(stmt, idx, value) raise "Db.bind failed (21): cannot bind a replay cursor" if stmt.is_a?(Integer) - current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_int64(stmt, idx, value)) + if value.nil? + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_null(stmt, idx)) + else + bind_text(stmt, idx, value) + end + end + + def self.bind_bool_opt(stmt, idx, value) + raise "Db.bind failed (21): cannot bind a replay cursor" if stmt.is_a?(Integer) + if value.nil? + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_null(stmt, idx)) + else + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_int64(stmt, idx, value ? 1 : 0)) + end end def self.bind_text(stmt, idx, value) raise "Db.bind failed (21): cannot bind a replay cursor" if stmt.is_a?(Integer) + # This shim's inline writer uses TEXT regardless of Ruby encoding. + # Preserve all bytes at the FFI boundary, including embedded NULs. current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_text(stmt, idx, value, value.bytesize, -1)) end @@ -1790,7 +1856,11 @@ def self.bind_text(stmt, idx, value) # form and the INTEGER affinity `t.boolean` columns get. def self.bind_bool(stmt, idx, value) raise "Db.bind failed (21): cannot bind a replay cursor" if stmt.is_a?(Integer) - current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_int64(stmt, idx, value ? 1 : 0)) + if value.nil? + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_null(stmt, idx)) + else + current_conn.bind_checked(stmt, idx, SQL.sqlite3_bind_int64(stmt, idx, value ? 1 : 0)) + end end def self.last_insert_rowid diff --git a/runtime/spinel/db_cruby.rb b/runtime/spinel/db_cruby.rb index 5e82206f1..cd0e7a232 100644 --- a/runtime/spinel/db_cruby.rb +++ b/runtime/spinel/db_cruby.rb @@ -738,10 +738,31 @@ def self.prepare(sql) end handle = { stmt: stmt, row: nil, cached: cached, capture: nil, open: open } open[stmt] = handle + handle[:capture] = { rows: [], names: stmt.columns, eof: false, sql: sql } if !qcache.nil? && !parameterized + handle + end + + # Explicit uncached reads skip statement reuse, but the separate + # request result cache still applies. Partial replays already promote + # to a transient statement, which finalize closes. + def self.prepare_uncached(sql) qcache = Fiber[:rh_qcache] - unless qcache.nil? || parameterized - handle[:capture] = { rows: [], names: stmt.columns, eof: false, sql: sql } + parameterized = sql.include?("?") + if !qcache.nil? && !parameterized && (hit = qcache[sql]) + return { stmt: nil, row: nil, cached: false, replay: hit, pos: 0, sql: sql } end + record_query(sql) + conn = current_dbh + stmt = conn.prepare(sql) + statement_handle(open_statements(conn), stmt, sql, false, !qcache.nil? && !parameterized) + end + + # Transient handles use the same ownership and bounded capture contract. + # The cached path constructs its handle inline on the query hot path. + def self.statement_handle(open, stmt, sql, cached, capture_rows) + handle = { stmt: stmt, row: nil, cached: cached, capture: nil, open: open } + open[stmt] = handle + handle[:capture] = { rows: [], names: stmt.columns, eof: false, sql: sql } if capture_rows handle end @@ -932,12 +953,37 @@ def self.bind_int(handle, idx, value) bind_value(handle, idx, value) end + def self.bind_int_opt(handle, idx, value) + bind_int(handle, idx, value) + end + + def self.bind_text_opt(handle, idx, value) + value.nil? ? bind_value(handle, idx, nil) : bind_text(handle, idx, value) + end + + def self.bind_bool_opt(handle, idx, value) + bind_value(handle, idx, value.nil? ? nil : (value ? 1 : 0)) + end + def self.bind_text(handle, idx, value) - bind_value(handle, idx, value) + value = value.to_s + # Match escape_string's storage class exactly. The gem otherwise binds + # every BINARY string as BLOB and every UTF-8 string (even NUL) as TEXT; + # SQLite equality does not equate the same bytes across those classes. + if value.include?("\0") || (value.encoding == Encoding::BINARY && !value.ascii_only?) + bind_value(handle, idx, value.b) + elsif value.encoding == Encoding::BINARY + bind_value(handle, idx, value.encode(Encoding::UTF_8)) + else + bind_value(handle, idx, value) + end + rescue StandardError => error + # Encoding checks/conversion can raise before bind_value is entered. + statement_failed(handle, "bind", error) end def self.bind_bool(handle, idx, value) - bind_value(handle, idx, value ? 1 : 0) + bind_value(handle, idx, value.nil? ? nil : (value ? 1 : 0)) end def self.last_insert_rowid diff --git a/runtime/spinel/db_jruby.rb b/runtime/spinel/db_jruby.rb index ef1481191..6aa778b69 100644 --- a/runtime/spinel/db_jruby.rb +++ b/runtime/spinel/db_jruby.rb @@ -358,6 +358,28 @@ def self.prepare(sql) st end + # Explicit uncached reads skip statement reuse, retaining result replay. + # finalize closes real transient statements, including replay promotion. + def self.prepare_uncached(sql) + qcache = Fiber[:rh_qcache] + parameterized = sql.include?("?") + if !qcache.nil? && !parameterized && (hit = qcache[sql]) + st = Stmt.new(nil, false) + st.sql = sql + st.replay = hit + return st + end + record_query(sql) + conn = current_dbh + pstmt = conn.raw.prepare_statement(sql) + st = Stmt.new(pstmt, false) + st.open = conn.open_statements + st.open[pstmt] = st + st.sql = sql + st.capture = { rows: [], names: nil, eof: false } if !qcache.nil? && !parameterized + st + end + # Run the query exactly once, lazily. `sqlite_adapter.rb`'s `select_rows` # calls `column_count` before the first `step?`, so either entry point # may be first to need a live ResultSet — execute on whichever wins and @@ -615,6 +637,70 @@ def self.statement_failed(stmt, _operation, error) raise error end + # Optional read predicates occupy one slot whether nil or present. + def self.bind_int_opt(handle, idx, value) + ps = handle.pstmt + raise "statement is not bindable" if ps.nil? || handle.executed + if value.nil? + ps.set_null(idx, Java::JavaSql::Types::INTEGER) + else + ps.set_long(idx, value) + end + rescue StandardError => error + statement_failed(handle, "bind", error) + end + + def self.bind_text_opt(handle, idx, value) + ps = handle.pstmt + raise "statement is not bindable" if ps.nil? || handle.executed + if value.nil? + ps.set_null(idx, Java::JavaSql::Types::VARCHAR) + else + bind_text(handle, idx, value) + end + rescue StandardError => error + statement_failed(handle, "bind", error) + end + + def self.bind_bool_opt(handle, idx, value) + ps = handle.pstmt + raise "statement is not bindable" if ps.nil? || handle.executed + if value.nil? + ps.set_null(idx, Java::JavaSql::Types::INTEGER) + else + ps.set_long(idx, value ? 1 : 0) + end + rescue StandardError => error + statement_failed(handle, "bind", error) + end + + # Match this shim's inline writer, including ASCII-only BINARY strings + # remaining TEXT. set_bytes keeps NUL and non-ASCII binary data out of + # Java String decoding; both JDBC setters copy the Ruby value at bind time. + def self.bind_text(stmt, idx, value) + pstmt = stmt.pstmt + return nil if pstmt.nil? + value = value.to_s + if value.include?("\0") || (value.encoding == Encoding::BINARY && !value.ascii_only?) + pstmt.set_bytes(idx, value.to_java_bytes) + else + pstmt.set_string(idx, value) + end + rescue StandardError => error + statement_failed(stmt, "bind", error) + end + + # SQLite boolean values are integers, with NULL distinct from false/0. + def self.bind_bool(stmt, idx, value) + pstmt = stmt.pstmt + return nil if pstmt.nil? + if value.nil? + pstmt.set_null(idx, Java::JavaSql::Types::INTEGER) + else + pstmt.set_int(idx, value ? 1 : 0) + end + end + def self.last_insert_rowid st = current_dbh.raw.create_statement rs = st.execute_query("SELECT last_insert_rowid()") diff --git a/scripts/ci-plan.py b/scripts/ci-plan.py index a8628194a..c10788588 100755 --- a/scripts/ci-plan.py +++ b/scripts/ci-plan.py @@ -41,12 +41,13 @@ # must not fail the Ruby PR floor. PUBLICATION = [*BASE, "compare", "browser-smoke-typescript"] CORE = ["build-spinel", "toolchain-spinel", "compare-spinel"] +PARAM_BIND_TESTS = ["param_binds", "param_binds_values", "param_binds_planner", "param_binds_cleanup"] SPINEL_TESTS = [ "date_columns_spinel", "framework_tests_spinel", "spinel_web_push_crypto", "spinel_db_lease", - "param_binds", + *PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", "spinel_param_builder", @@ -106,17 +107,23 @@ def native_coverage(path): suites.add(focused[1]) if path == "tests/support/db_concurrency_spinel.rb": suites.add("db_sqlite_concurrency") + # Gate drivers stay flat beside their Rust harness. Match the most + # specific suite first (e.g. param_binds_values before param_binds). + if path == "tests/param_binds_text_cleanup.rb": + suites.add("param_binds_cleanup") + elif path.startswith("tests/") and path.endswith(".rb"): + stem = path[len("tests/"):-len(".rb")] + for suite in reversed(PARAM_BIND_TESTS): + if stem == suite or stem.startswith(suite + "_"): + suites.add(suite) + break + if path == "runtime/spinel/test/statement_cache_cases.rb": + suites.add("param_binds") if path in { - "tests/param_binds_emit.rb", - "tests/param_binds_raw_where.rb", - "tests/param_binds_runtime.rb", - "tests/param_binds_cruby_cache.rb", - "tests/param_binds_spinel_cache.rb", - "runtime/spinel/test/statement_cache_cases.rb", "tests/support/emit_and_run.rs", "src/lower/model_to_library/adapter_emit.rs", - } or path.startswith("src/lower/arel/"): - suites.add("param_binds") + } or path.startswith(("src/lower/arel/", "src/lower/model_to_library/adapter_emit/")): + suites.update(PARAM_BIND_TESTS) if path.startswith(("runtime/spinel/", "runtime/ruby/")) and not interpreter_only: name = path.rsplit("/", 1)[-1] owned_tests = set() @@ -143,7 +150,7 @@ def native_coverage(path): owned_tests.update( ( "spinel_db_lease", - "param_binds", + *PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ) diff --git a/src/emit/ruby.rs b/src/emit/ruby.rs index df90efda2..6f354951b 100644 --- a/src/emit/ruby.rs +++ b/src/emit/ruby.rs @@ -218,13 +218,14 @@ pub(crate) fn materialize_models( ) .0, ); - let lcs = crate::lower::model_to_library::lower_models_inner( + let lcs = crate::lower::model_to_library::lower_models_inner_with_ruby_values( &app.models, &app.schema, Vec::new(), ¶ms_specs, &assoc_scopes, materialization, + true, ).0; (lcs, params_specs) } @@ -576,6 +577,7 @@ fn lower_controllers_for_spinel(app: &App, format_breadth: FormatBreadth) -> Vec &app.controllers, model_extras, crate::lower::controller_to_library::LowerControllerOptions { + ruby_read_values: true, schema: Some(&app.schema), views: &app.views, library_classes: &app.library_classes, diff --git a/src/emit/ruby/library.rs b/src/emit/ruby/library.rs index dd79550df..401003a9d 100644 --- a/src/emit/ruby/library.rs +++ b/src/emit/ruby/library.rs @@ -5198,6 +5198,29 @@ pub(crate) fn apply_hydration_nil_lowering(lcs: &mut [LibraryClass], app: &App) if let Some(lc) = lcs.iter_mut().find(|lc| lc.name == model.name) { for m in &mut lc.methods { widen_fk_zero_guards(&mut m.body, &nullable_fks); + // Polymorphic readers dispatch on the type discriminator, + // so they have no zero-sentinel guard to widen. Guard their + // nullable FK here too, before any key-typed adapter call. + if m.name_span.is_synthetic() { + for assoc in model.associations() { + if let crate::dialect::Association::BelongsTo { + name, foreign_key, polymorphic: true, .. + } = assoc { + if m.name == *name && nullable.contains(foreign_key) { + let cond = Expr::new(Span::synthetic(), ExprNode::Send { + recv: Some(Expr::new(Span::synthetic(), ExprNode::Ivar { name: foreign_key.clone() })), + method: Symbol::from("nil?"), args: vec![], block: None, + parenthesized: false, + }); + m.body = Expr::new(m.body.span, ExprNode::If { + cond, + then_branch: Expr::new(Span::synthetic(), ExprNode::Lit { value: Literal::Nil }), + else_branch: m.body.clone(), + }); + } + } + } + } } } } @@ -5276,10 +5299,8 @@ fn widen_fk_zero_guards(expr: &mut Expr, fks: &BTreeSet) { if method.as_str() == "==" && args.len() == 1 => { matches!(&*r.node, ExprNode::Ivar { name } if fks.contains(name)) - && matches!( - &*args[0].node, - ExprNode::Lit { value: Literal::Int { value: 0 } } - ) + && (matches!(&*args[0].node, ExprNode::Lit { value: Literal::Int { value: 0 } }) + || matches!(&*args[0].node, ExprNode::Lit { value: Literal::Str { value } } if value.is_empty())) } _ => false, }; @@ -5835,6 +5856,67 @@ pub(super) fn emit_library_class_pair_with_synthesized( vec![rb, rbs] } +/// A lowered read owns its statement from prepare until finalize, including +/// argument serialization and row hydration. Release within that scope even +/// when the caller rescues inside a longer connection lease. This Ruby-only +/// pass leaves strict-target IR unchanged and covers reloads and preloads too. +fn read_statement_cleanup(lc: &LibraryClass) -> std::borrow::Cow<'_, LibraryClass> { + fn db_send(expr: &Expr, name: &str) -> bool { + matches!(&*expr.node, ExprNode::Send { recv: Some(recv), method, .. } + if method.as_str() == name && matches!(&*recv.node, + ExprNode::Const { path } if path.len() == 1 && path[0].as_str() == "Db")) + } + fn prepare(expr: &Expr) -> bool { + db_send(expr, "prepare") || db_send(expr, "prepare_uncached") + } + fn prepared_name(expr: &Expr) -> Option<&Symbol> { + match &*expr.node { + ExprNode::Assign { target: LValue::Var { name, .. }, value } if prepare(value) => Some(name), + _ => None, + } + } + fn finalize(expr: &Expr, name: &Symbol) -> bool { + db_send(expr, "finalize") && matches!(&*expr.node, + ExprNode::Send { args, .. } if args.len() == 1 + && matches!(&*args[0].node, ExprNode::Var { name: actual, .. } if actual == name)) + } + fn needs_cleanup(expr: &Expr) -> bool { + let mut found = prepared_name(expr).is_some(); + expr.node.for_each_child(&mut |child| found |= needs_cleanup(child)); + found + } + fn rewrite(expr: &mut Expr) { + expr.node.for_each_child_mut(&mut rewrite); + let ExprNode::Seq { exprs } = expr.node.as_mut() else { return }; + let mut start = 0; + while start < exprs.len() { + if let Some(name) = prepared_name(&exprs[start]) { + if let Some(end) = (start + 1..exprs.len()).find(|&i| finalize(&exprs[i], name)) { + let mut work: Vec<_> = exprs.drain(start + 1..=end).collect(); + let cleanup = work.pop().unwrap(); + let body = Expr::new(Span::synthetic(), ExprNode::Seq { exprs: work }); + exprs.insert(start + 1, Expr::new(Span::synthetic(), ExprNode::BeginRescue { + body, + rescues: vec![], + else_branch: None, + ensure: Some(cleanup), + implicit: false, + })); + } + } + start += 1; + } + } + if !lc.methods.iter().any(|m| needs_cleanup(&m.body)) { + return std::borrow::Cow::Borrowed(lc); + } + let mut adapted = lc.clone(); + for method in &mut adapted.methods { + rewrite(&mut method.body); + } + std::borrow::Cow::Owned(adapted) +} + /// Emit a group of LibraryFunctions sharing a `module_path` as a /// single Ruby file. Mirrors `typescript::library::emit_module_file` /// — converts the function group into a synthetic @@ -5985,6 +6067,8 @@ pub(super) fn emit_library_class_decl_with_synthesized( out_path: PathBuf, synthesized_siblings: &[(String, String)], ) -> EmittedFile { + let guarded = read_statement_cleanup(lc); + let lc = guarded.as_ref(); // The one chokepoint every library-shape file goes through, and the // only place that knows which class is being emitted — the send // emitter is a free function reached from a dozen callers. A reopen diff --git a/src/lower/arel/ir.rs b/src/lower/arel/ir.rs index c7515c124..8341dc2d9 100644 --- a/src/lower/arel/ir.rs +++ b/src/lower/arel/ir.rs @@ -212,6 +212,9 @@ pub enum ColumnSpec { #[derive(Clone, Debug)] pub enum Predicate { Eq(ColRef, Value), + /// Ruby-family runtime equality: select `= value` or `IS NULL` from + /// the value, preserving SQLite partial-index and join optimizations. + NullableEq(ColRef, Value), And(Box, Box), Or(Box, Box), } @@ -268,9 +271,8 @@ pub enum ValueType { } impl ValueType { - /// True for the nullable variants. Callers that cannot express - /// NULL (the placeholder-bind path has no `bind_null`) route these - /// through inline escaping instead. + /// True for the nullable variants. Optional binds and inline escapes + /// both preserve nil as SQL NULL. pub fn is_nullable(self) -> bool { matches!(self, ValueType::IntOpt | ValueType::StrOpt | ValueType::FloatOpt | ValueType::BoolOpt) } diff --git a/src/lower/arel/mod.rs b/src/lower/arel/mod.rs index e2599b29c..6be264bde 100644 --- a/src/lower/arel/mod.rs +++ b/src/lower/arel/mod.rs @@ -17,6 +17,7 @@ pub mod build; pub mod ir; pub mod visitor; +mod ruby_values; pub use build::{try_build_arel, try_build_arel_with_assocs}; pub use ir::{ @@ -61,6 +62,17 @@ pub fn rewrite_arel_in_expr_with_assocs( schema: &Schema, registry: &HashMap, assocs: &[crate::lower::model_associations::AssociationEdge], +) -> bool { + rewrite_arel_in_expr_with_ruby_values(expr, schema, registry, assocs, false) +} + +/// Ruby-family values preserve SQL NULL without changing strict-target emit. +pub(crate) fn rewrite_arel_in_expr_with_ruby_values( + expr: &mut Expr, + schema: &Schema, + registry: &HashMap, + assocs: &[crate::lower::model_associations::AssociationEdge], + ruby_read_values: bool, ) -> bool { // Names (ivars/locals) the body later refines with relation-chain // methods (`@moderations.where(...)` after `@moderations = @@ -69,7 +81,7 @@ pub fn rewrite_arel_in_expr_with_assocs( // runtime Relation path. let mut refined = std::collections::HashSet::new(); collect_relation_refined_names(expr, &mut refined); - let mut changed = rewrite_arel_inner(expr, schema, registry, assocs, &refined); + let mut changed = rewrite_arel_inner(expr, schema, registry, assocs, &refined, ruby_read_values); // Both call sites hand us a METHOD BODY, and a body that is a // single statement is not a `Seq` — so the hoist post-pass inside // `rewrite_arel_inner`, which walks a Seq's statement list, had no @@ -192,6 +204,7 @@ fn rewrite_arel_inner( registry: &HashMap, assocs: &[crate::lower::model_associations::AssociationEdge], refined: &std::collections::HashSet, + ruby_read_values: bool, ) -> bool { if let ExprNode::Assign { target, .. } = expr.node.as_ref() { let name = match target { @@ -204,9 +217,12 @@ fn rewrite_arel_inner( } } if let ExprNode::Send { .. } = expr.node.as_ref() { - if let Some((op, owner)) = + if let Some((mut op, owner)) = build::try_build_arel_with_assocs(expr, schema, registry, assocs) { + if ruby_read_values { + ruby_values::normalize(&mut op, schema); + } let mut replacement = SqliteVisitor.visit(&op, schema, &owner); // The expansion replaces the recognized chain wholesale; // its provenance is the chain call site. Subtrees the @@ -241,15 +257,15 @@ fn rewrite_arel_inner( let ExprNode::Send { recv: Some(recv), args, .. } = &mut *expr.node else { unreachable!("matched Send with recv above"); }; - let mut changed = rewrite_arel_spine_args(recv, schema, registry, assocs, refined); + let mut changed = rewrite_arel_spine_args(recv, schema, registry, assocs, refined, ruby_read_values); for a in args { - changed |= rewrite_arel_inner(a, schema, registry, assocs, refined); + changed |= rewrite_arel_inner(a, schema, registry, assocs, refined, ruby_read_values); } return changed; } let mut changed = false; walk_subexprs_mut(expr, &mut |e| { - changed |= rewrite_arel_inner(e, schema, registry, assocs, refined) + changed |= rewrite_arel_inner(e, schema, registry, assocs, refined, ruby_read_values) }); // Post-pass: when an Arel rewrite landed a multi-stmt hydrate Seq // in a *value* position — directly as an Assign value @@ -281,17 +297,18 @@ fn rewrite_arel_spine_args( registry: &HashMap, assocs: &[crate::lower::model_associations::AssociationEdge], refined: &std::collections::HashSet, + ruby_read_values: bool, ) -> bool { let mut changed = false; if let ExprNode::Send { recv, args, block, .. } = &mut *expr.node { if let Some(r) = recv { - changed |= rewrite_arel_spine_args(r, schema, registry, assocs, refined); + changed |= rewrite_arel_spine_args(r, schema, registry, assocs, refined, ruby_read_values); } for a in args { - changed |= rewrite_arel_inner(a, schema, registry, assocs, refined); + changed |= rewrite_arel_inner(a, schema, registry, assocs, refined, ruby_read_values); } if let Some(b) = block { - changed |= rewrite_arel_inner(b, schema, registry, assocs, refined); + changed |= rewrite_arel_inner(b, schema, registry, assocs, refined, ruby_read_values); } } changed diff --git a/src/lower/arel/ruby_values.rs b/src/lower/arel/ruby_values.rs new file mode 100644 index 000000000..e3b51670f --- /dev/null +++ b/src/lower/arel/ruby_values.rs @@ -0,0 +1,108 @@ +//! Ruby-family read values. Strict targets retain their existing value +//! representation until their nullable runtime and bind capabilities land. + +use crate::expr::{Expr, ExprNode, Literal}; +use crate::ident::Symbol; +use crate::schema::{ColumnType, Schema}; +use crate::ty::Ty; + +use super::ir::{ArelOp, Predicate, Value, ValueType}; + +pub(super) fn normalize(op: &mut ArelOp, schema: &Schema) { + if let ArelOp::Select(select) = op { + if let Some(predicate) = &mut select.conditions { + normalize_predicate(predicate, schema); + } + } +} + +fn normalize_predicate(predicate: &mut Predicate, schema: &Schema) { + match predicate { + Predicate::And(left, right) | Predicate::Or(left, right) => { + normalize_predicate(left, schema); + normalize_predicate(right, schema); + } + Predicate::Eq(col, Value::Runtime { expr, ty }) => { + let column = schema.tables.get(&col.table.0) + .and_then(|table| table.columns.iter().find(|c| c.name == col.column)); + let nullable_column = column.is_some_and(|column| column.nullable && !column.primary_key); + let nullable_value = expr.ty.as_ref().is_some_and(is_nullable); + let nullable = nullable_column || nullable_value; + if let Some(column) = column { + // A nullable column does not make a scalar RHS optional: + // Float still needs to_s; only an optional RHS preserves nil. + *expr = serialized_value_expr(expr, &column.col_type, nullable_value); + } + if nullable { + *ty = match ty { + ValueType::Int | ValueType::IntOpt => ValueType::IntOpt, + ValueType::Str | ValueType::StrOpt => ValueType::StrOpt, + ValueType::Bool | ValueType::BoolOpt => ValueType::BoolOpt, + ValueType::FloatOpt => ValueType::FloatOpt, + }; + if nullable_column { + *predicate = Predicate::NullableEq(col.clone(), Value::Runtime { expr: expr.clone(), ty: *ty }); + } + } + } + Predicate::Eq(_, _) | Predicate::NullableEq(_, _) => {} + } +} + +/// Only native temporal values need the writer's formatter. A String ivar +/// (for example a date filter from params) already contains SQL text and +/// must stay a String. Explicit numeric conversion preserves nil; an IR +/// narrowing Cast can be elided after later typing and is not serialization. +fn serialized_value_expr(expr: &Expr, column: &ColumnType, nullable: bool) -> Expr { + let text_ty = if nullable { Ty::Union { variants: vec![Ty::Str, Ty::Nil] } } else { Ty::Str }; + let converted = match column { + ColumnType::Float | ColumnType::Decimal { .. } => { + let text = Expr::new(expr.span, ExprNode::Send { + recv: Some(expr.clone()), method: Symbol::from("to_s"), args: vec![], + block: None, parenthesized: false, + }); + if nullable { + // The builder admits only pure ivar reads here. Keep this + // semantic conversion explicit through subsequent typing. + Expr::new(expr.span, ExprNode::If { + cond: Expr::new(expr.span, ExprNode::Send { + recv: Some(expr.clone()), method: Symbol::from("nil?"), args: vec![], + block: None, parenthesized: false, + }), + then_branch: Expr::new(expr.span, ExprNode::Lit { value: Literal::Nil }), + else_branch: text, + }) + } else { + text + } + } + ColumnType::Date | ColumnType::DateTime | ColumnType::Time => { + let method = match expr.ty.as_ref().map(Ty::peel_nilable) { + Some(Ty::Time) => "format_db_time", + Some(Ty::Date) => "format_db_date", + // App RBS signatures may still spell the native type as + // a root class; nominal namespaced classes are not Time. + Some(Ty::Class { id, args }) if args.is_empty() && id.0.as_str() == "Time" => "format_db_time", + Some(Ty::Class { id, args }) if args.is_empty() && id.0.as_str() == "Date" => "format_db_date", + _ => return expr.clone(), + }; + Expr::new(expr.span, ExprNode::Send { + recv: Some(Expr::new(expr.span, ExprNode::Const { path: vec![Symbol::from("ActiveSupport")] })), + method: Symbol::from(method), args: vec![expr.clone()], + block: None, parenthesized: true, + }) + } + ColumnType::Integer | ColumnType::BigInt | ColumnType::Reference { .. } + | ColumnType::Boolean | ColumnType::String { .. } | ColumnType::Text + | ColumnType::Binary | ColumnType::Json | ColumnType::Uuid => return expr.clone(), + }; + crate::lower::typing::with_ty(converted, text_ty) +} + +fn is_nullable(ty: &Ty) -> bool { + match ty { + Ty::Nil => true, + Ty::Union { variants } => variants.iter().any(is_nullable), + _ => false, + } +} diff --git a/src/lower/arel/visitor.rs b/src/lower/arel/visitor.rs index c2d8f9494..512fd5947 100644 --- a/src/lower/arel/visitor.rs +++ b/src/lower/arel/visitor.rs @@ -53,42 +53,106 @@ pub(crate) fn param_binds_enabled() -> bool { .unwrap_or(false) } -/// One deferred bind: the unrendered value expr plus the `ValueType` -/// that picks `bind_int` / `bind_text` / `bind_bool`. Accumulated by the -/// SQL composers alongside the `?` they emit, then drained into -/// `Db.bind_*(stmt, i, expr)` calls right after the `Db.prepare`. +/// A typed pending bind. Nullable predicates also select an SQL fragment. +/// Their runtime branch reserves a position together with the `= ?` fragment; +/// position zero means `IS NULL` and no bind. The RHS is evaluated just once. struct Bind { expr: Expr, ty: ValueType, + nullable_column: Option, + parameterized: bool, } -/// `Db.bind_(stmt, <1-based idx>, )` for each accumulated -/// bind, in placeholder order (sqlite bind indices are 1-based). The -/// per-value type is known at composition time, so each bind is -/// monomorphic — no heterogeneous bind bag on the hot path. -fn emit_bind_calls(stmt: &Symbol, binds: &[Bind]) -> Vec { +fn bind_local(kind: &str, i: usize) -> Symbol { + Symbol::from(format!("__rh_{}_{}", kind, i)) +} + +fn has_shaped_predicates(binds: &[Bind]) -> bool { + binds.iter().any(|b| b.nullable_column.is_some()) +} + +/// No exponential code generation: one branch per nullable predicate. Up to +/// seven predicates have at most 128 SQL shapes; larger combinations bypass +/// the cache, including Spinel's cache which only trims at lease boundaries. +const MAX_CACHED_NULLABLE_PREDICATES: usize = 7; + +fn emit_prepare(stmt: &Symbol, sql: Expr, binds: &[Bind]) -> Vec { let db = ClassId(Symbol::from(DB_MOD)); - binds - .iter() - .enumerate() - .map(|(i, b)| { - let method = match b.ty { - ValueType::Int => "bind_int", - ValueType::Str => "bind_text", - ValueType::Bool => "bind_bool", - // Nullable values never reach here: `push_value_segment` - // routes them through inline escaping, since the bind - // surface has no `bind_null`. - ty if ty.is_nullable() => unreachable!("nullable value in bind path: {ty:?}"), - _ => unreachable!(), + let mut out = Vec::new(); + let shaped = has_shaped_predicates(binds); + let index = Symbol::from("__rh_bind_index"); + if shaped && binds.iter().any(|b| b.parameterized) { + out.push(assign_var(&index, lit_int(0))); + } + for (i, b) in binds.iter().enumerate() { + if !shaped { break; } + // Reserving the slot is the bind decision. The later typed bind + // consumes this position, never re-evaluating a nullable RHS. + let reserve = || vec![ + assign_var(&index, send_to(var_ref(&index), "+", vec![lit_int(1)], false)), + assign_var(&bind_local("position", i), var_ref(&index)), + ]; + if let Some(column) = &b.nullable_column { + let value = bind_local("value", i); + let fragment = bind_local("predicate", i); + out.push(assign_var(&value, b.expr.clone())); + if b.parameterized { + out.push(assign_var(&bind_local("position", i), lit_int(0))); + } + let non_nil_sql = if b.parameterized { + lit_str(format!("{} = ?", column)) + } else { + concat_chain(vec![lit_str(format!("{} = ", column)), escape_value(&db, + &Value::Runtime { expr: var_ref(&value), ty: b.ty })]) }; - db_call( - &db, - method, - vec![var_ref(stmt), lit_int((i + 1) as i64), b.expr.clone()], - ) - }) - .collect() + let mut non_nil = vec![assign_var(&fragment, non_nil_sql)]; + if b.parameterized { non_nil.extend(reserve()); } + out.push(Expr::new(Span::synthetic(), ExprNode::If { + cond: send_to(var_ref(&value), "nil?", vec![], false), + then_branch: assign_var(&fragment, lit_str(format!("{} IS NULL", column))), + else_branch: seq(non_nil), + })); + } else { + out.push(assign_var(&bind_local("value", i), b.expr.clone())); + out.extend(reserve()); + } + } + let nullable_count = binds.iter().filter(|b| b.nullable_column.is_some()).count(); + let prepare = if nullable_count > MAX_CACHED_NULLABLE_PREDICATES && binds.iter().any(|b| b.parameterized) { + "prepare_uncached" + } else { + "prepare" + }; + out.push(assign_var(stmt, db_call(&db, prepare, vec![sql]))); + out +} + +/// Fixed-shape queries keep literal positions. Shaped queries consume the +/// positions reserved during SQL construction, skipping nil predicates. +fn emit_bind_calls(stmt: &Symbol, binds: &[Bind]) -> Vec { + let db = ClassId(Symbol::from(DB_MOD)); + let shaped = has_shaped_predicates(binds); + binds.iter().enumerate().filter(|(_, b)| b.parameterized).map(|(i, b)| { + let method = match b.ty { + ValueType::Int => "bind_int", + ValueType::Str => "bind_text", + ValueType::Bool => "bind_bool", + ValueType::IntOpt => "bind_int_opt", + ValueType::StrOpt => "bind_text_opt", + ValueType::BoolOpt => "bind_bool_opt", + ValueType::FloatOpt => unreachable!("unserialized float in bind path"), + }; + let position = if shaped { var_ref(&bind_local("position", i)) } else { lit_int((i + 1) as i64) }; + let value = if shaped { var_ref(&bind_local("value", i)) } else { b.expr.clone() }; + let call = db_call(&db, method, vec![var_ref(stmt), position.clone(), value]); + if b.nullable_column.is_some() { + Expr::new(Span::synthetic(), ExprNode::If { + cond: send_to(position, ">", vec![lit_int(0)], false), + then_branch: call, + else_branch: nil_lit(), + }) + } else { call } + }).collect() } /// Render an Arel tree into an `Expr` that calls into the per-target @@ -173,7 +237,7 @@ fn emit_single_hydrate(sel: &Select, table: &Table, owner: &ClassId, param: bool let mut binds = Vec::new(); let sql = compose_sql_select(sel, table, param, &mut binds); - let stmt_assign = assign_var(&stmt, db_call(&db, "prepare", vec![sql])); + let prepare = emit_prepare(&stmt, sql, &binds); let result_init = assign_var(&result, nil_lit()); // if Db.step?(stmt) ; result = .from_stmt(stmt) ; end @@ -193,7 +257,7 @@ fn emit_single_hydrate(sel: &Select, table: &Table, owner: &ClassId, param: bool let finalize = db_call(&db, "finalize", vec![var_ref(&stmt)]); // stmt = prepare ; [bind …] ; result = nil ; if step? {…} ; finalize ; result - let mut stmts = vec![stmt_assign]; + let mut stmts = prepare; stmts.extend(emit_bind_calls(&stmt, &binds)); stmts.push(result_init); stmts.push(if_expr); @@ -217,7 +281,7 @@ fn emit_multi_hydrate( let mut binds = Vec::new(); let sql = compose_sql_select(sel, table, param, &mut binds); - let stmt_assign = assign_var(&stmt, db_call(&db, "prepare", vec![sql])); + let prepare = emit_prepare(&stmt, sql, &binds); // Empty Array literal carries an explicit `Array` type // annotation so strict-target emit (Crystal `[] of Owner`) // matches the subsequent `results << instance` push semantics. @@ -271,7 +335,7 @@ fn emit_multi_hydrate( // hydrate loop. Preload sub-queries build their own statements and // keep the inline `IN (…)` list (variable arity — not parameterized // here; see the note in `push_preload_stmts`). - let mut stmts = vec![stmt_assign]; + let mut stmts = prepare; stmts.extend(emit_bind_calls(&stmt, &binds)); stmts.push(results_init); stmts.push(while_loop); @@ -580,7 +644,7 @@ fn emit_pluck(sel: &Select, table: &Table, col: &super::ir::ColRef, param: bool) let mut binds = Vec::new(); let sql = compose_sql_select(sel, table, param, &mut binds); - let stmt_assign = assign_var(&stmt, db_call(&db, "prepare", vec![sql])); + let prepare = emit_prepare(&stmt, sql, &binds); // The element type is carried explicitly for the same reason the // hydrate loop carries `Array`: an untyped `[]` types as @@ -613,7 +677,7 @@ fn emit_pluck(sel: &Select, table: &Table, col: &super::ir::ColRef, param: bool) }, ); - let mut stmts = vec![stmt_assign]; + let mut stmts = prepare; stmts.extend(emit_bind_calls(&stmt, &binds)); stmts.push(results_init); stmts.push(while_loop); @@ -678,7 +742,7 @@ fn emit_group_count( let mut binds = Vec::new(); let sql = compose_sql_select(sel, table, param, &mut binds); - let stmt_assign = assign_var(&stmt, db_call(&db, "prepare", vec![sql])); + let prepare = emit_prepare(&stmt, sql, &binds); let results_init = assign_var( &results, @@ -716,7 +780,7 @@ fn emit_group_count( }, ); - let mut stmts = vec![stmt_assign]; + let mut stmts = prepare; stmts.extend(emit_bind_calls(&stmt, &binds)); stmts.push(results_init); stmts.push(while_loop); @@ -738,14 +802,14 @@ fn emit_count(sel: &Select, table: &Table, param: bool) -> Expr { let mut binds = Vec::new(); push_where_segments(&mut segments, sel.conditions.as_ref(), table, param, &mut binds); - let stmt_assign = assign_var(&stmt, db_call(&db, "prepare", vec![concat_chain(segments)])); + let prepare = emit_prepare(&stmt, concat_chain(segments), &binds); let step = db_call(&db, "step?", vec![var_ref(&stmt)]); let read = db_call(&db, "column_int", vec![var_ref(&stmt), lit_int(0)]); let result_assign = assign_var(&result, read); let finalize = db_call(&db, "finalize", vec![var_ref(&stmt)]); // stmt = prepare ; [bind …] ; step? ; result = column_int ; finalize ; result - let mut stmts = vec![stmt_assign]; + let mut stmts = prepare; stmts.extend(emit_bind_calls(&stmt, &binds)); stmts.push(step); stmts.push(result_assign); @@ -768,12 +832,12 @@ fn emit_exists(sel: &Select, table: &Table, param: bool) -> Expr { segments.push(lit_str(format!(" LIMIT {}", n))); } - let stmt_assign = assign_var(&stmt, db_call(&db, "prepare", vec![concat_chain(segments)])); + let prepare = emit_prepare(&stmt, concat_chain(segments), &binds); let result_assign = assign_var(&result, db_call(&db, "step?", vec![var_ref(&stmt)])); let finalize = db_call(&db, "finalize", vec![var_ref(&stmt)]); // stmt = prepare ; [bind …] ; result = step? ; finalize ; result - let mut stmts = vec![stmt_assign]; + let mut stmts = prepare; stmts.extend(emit_bind_calls(&stmt, &binds)); stmts.push(result_assign); stmts.push(finalize); @@ -973,6 +1037,15 @@ fn push_predicate_segments( Predicate::Eq(col, Value::LiteralNull) => { segments.push(lit_str(format!("{} IS NULL", crate::naming::sql_ident(col.column.as_str())))); } + Predicate::NullableEq(col, Value::Runtime { expr, ty }) => { + segments.push(var_ref(&bind_local("predicate", binds.len()))); + binds.push(Bind { + expr: expr.clone(), ty: *ty, + nullable_column: Some(crate::naming::sql_ident(col.column.as_str())), + parameterized: param && *ty != ValueType::FloatOpt, + }); + } + Predicate::NullableEq(_, _) => unreachable!("only runtime predicates select a shape"), Predicate::Eq(col, val) => { segments.push(lit_str(format!("{} = ", crate::naming::sql_ident(col.column.as_str())))); push_value_segment(segments, val, param, binds); @@ -1004,11 +1077,10 @@ fn push_value_segment(segments: &mut Vec, val: &Value, param: bool, binds: Value::LiteralStr(s) => segments.push(lit_str(format!("'{}'", s.replace('\'', "''")))), Value::LiteralBool(b) => segments.push(lit_str(if *b { "1".into() } else { "0".into() })), Value::LiteralNull => segments.push(lit_str("NULL".into())), - // A nullable value can't ride the placeholder path — there is - // no `bind_null` — so it always renders inline. - Value::Runtime { expr, ty } if param && !ty.is_nullable() => { + // One optional bind still occupies exactly one placeholder. + Value::Runtime { expr, ty } if param && *ty != ValueType::FloatOpt => { segments.push(lit_str("?".to_string())); - binds.push(Bind { expr: expr.clone(), ty: *ty }); + binds.push(Bind { expr: expr.clone(), ty: *ty, nullable_column: None, parameterized: true }); } Value::Runtime { .. } => segments.push(escape_value(&db, val)), } diff --git a/src/lower/controller_to_library/mod.rs b/src/lower/controller_to_library/mod.rs index 7e86eb4ce..ba2233816 100644 --- a/src/lower/controller_to_library/mod.rs +++ b/src/lower/controller_to_library/mod.rs @@ -247,6 +247,8 @@ fn returns_relation(ty: &Ty) -> bool { /// positional args. #[derive(Default)] pub struct LowerControllerOptions<'a> { + /// Ruby-family nullable read values; strict-target defaults stay unchanged. + pub ruby_read_values: bool, /// App `Schema` — enables the Arel SQL-chain lowering pass. pub schema: Option<&'a crate::schema::Schema>, /// App views — scanned for `*.json.jbuilder` format dispatch and the @@ -291,6 +293,7 @@ pub fn lower_controllers_with_arel_views_assocs_and_routes( opts: LowerControllerOptions, ) -> Vec { let LowerControllerOptions { + ruby_read_values, schema, views, library_classes, @@ -532,8 +535,8 @@ pub fn lower_controllers_with_arel_views_assocs_and_routes( let refined_across_methods = refined_result_methods.contains(&method.name); if let Some(schema) = schema { if !refined_across_methods { - rewritten |= crate::lower::arel::rewrite_arel_in_expr_with_assocs( - &mut method.body, schema, &classes, assocs, + rewritten |= crate::lower::arel::rewrite_arel_in_expr_with_ruby_values( + &mut method.body, schema, &classes, assocs, ruby_read_values, ); } } diff --git a/src/lower/model_to_library/mod.rs b/src/lower/model_to_library/mod.rs index db42da18b..504efd2f5 100644 --- a/src/lower/model_to_library/mod.rs +++ b/src/lower/model_to_library/mod.rs @@ -210,6 +210,20 @@ pub(crate) fn lower_models_inner( params_specs: &crate::lower::controller_to_library::params::ParamsSpecs, unfolded: &std::collections::HashSet<(ClassId, Symbol)>, materialization: Materialization<'_>, +) -> (Vec, HashMap) { + lower_models_inner_with_ruby_values( + models, schema, extra_class_infos, params_specs, unfolded, materialization, false, + ) +} + +pub(crate) fn lower_models_inner_with_ruby_values( + models: &[Model], + schema: &Schema, + extra_class_infos: Vec<(ClassId, crate::analyze::ClassInfo)>, + params_specs: &crate::lower::controller_to_library::params::ParamsSpecs, + unfolded: &std::collections::HashSet<(ClassId, Symbol)>, + materialization: Materialization<'_>, + ruby_read_values: bool, ) -> (Vec, HashMap) { let mut all_methods: Vec<(Vec, ClassId, Option<&Table>, &Model)> = Vec::new(); let mut classes: HashMap = HashMap::new(); @@ -347,7 +361,9 @@ pub(crate) fn lower_models_inner( let unfold = method.receiver == crate::dialect::MethodReceiver::Class && unfolded.contains(&(model.name.clone(), method.name.clone())); if !unfold { - crate::lower::arel::rewrite_arel_in_expr(&mut method.body, schema, &classes); + crate::lower::arel::rewrite_arel_in_expr_with_ruby_values( + &mut method.body, schema, &classes, &[], ruby_read_values, + ); } type_method_body(method, &classes, table, Some(model)); } diff --git a/src/project.rs b/src/project.rs index 163c84b27..7c89fa0d8 100644 --- a/src/project.rs +++ b/src/project.rs @@ -789,6 +789,34 @@ pub fn spinel_relation_model_handle(files: &mut [(String, String)]) -> Result<() *conn = conn.replace(narrow, wide); } *conn = widen_spinel_base_on_sigs(conn); + + // The public find/exists? nil guards live in the connection reopen. + // Spinel seeds their inherited adapter calls from Base's declaration, + // even when a String-keyed model overrides the adapter with a String + // argument. Follow the app-wide id contract established by + // widen_key_contract: Integer-only apps keep their original seeds, + // while apps with String keys need both kinds at shared dispatch. + // Each generated model still declares its schema's scalar. + let base = files + .iter_mut() + .find(|(p, _)| { + p == "sig/runtime/active_record/base.rbs" || p == "runtime/active_record/base.rbs" + }) + .ok_or_else(|| { + "spinel_relation_model_handle: active_record/base.rbs not in the tree".to_string() + })?; + if base.1.contains(" def id: () -> (Integer | String)\n") { + for method in ["self._adapter_find_by_id", "self._adapter_exists_by_id?"] { + let narrow = format!(" def {method}: (Integer id)"); + let wide = format!(" def {method}: (Integer | String id)"); + if !base.1.contains(&narrow) { + return Err(format!( + "spinel_relation_model_handle: base.rbs no longer declares {narrow:?}" + )); + } + base.1 = base.1.replace(&narrow, &wide); + } + } resolve_runtime_sig_conflicts(files) } diff --git a/tests/ci_plan_test.py b/tests/ci_plan_test.py index 4005228a1..6dad17943 100644 --- a/tests/ci_plan_test.py +++ b/tests/ci_plan_test.py @@ -402,11 +402,16 @@ def test_param_binds_owns_lowering_drivers_and_database_runtime(self): "tests/param_binds_runtime.rb", "tests/param_binds_cruby_cache.rb", "tests/param_binds_spinel_cache.rb", + "tests/param_binds_associations.rb", + "tests/param_binds_nil.rb", "tests/support/emit_and_run.rs", ]: with self.subTest(path=path): plan = ci.select([path]) - self.assertEqual(plan["spinel_tests"], ["param_binds"]) + self.assertEqual( + plan["spinel_tests"], + ci.PARAM_BIND_TESTS if path.startswith("src/") or path == "tests/support/emit_and_run.rs" else ["param_binds"], + ) self.assertEqual( self.extras(plan), set(ci.CORE) | {"framework-tests-spinel"} ) @@ -414,7 +419,7 @@ def test_param_binds_owns_lowering_drivers_and_database_runtime(self): ci.select(["runtime/spinel/db.rb"])["spinel_tests"], [ "spinel_db_lease", - "param_binds", + *ci.PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ], @@ -424,7 +429,7 @@ def test_param_binds_owns_lowering_drivers_and_database_runtime(self): [ "date_columns_spinel", "spinel_db_lease", - "param_binds", + *ci.PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ], @@ -455,6 +460,14 @@ def test_jdbc_probes_select_the_existing_comparison_without_archives(self): self.assertEqual(plan["smoke"], []) self.assertEqual(plan["archives"], []) + def test_param_bind_suite_drivers_select_their_native_harness(self): + for suite in ci.PARAM_BIND_TESTS: + for suffix in (".rs", ".rb", "_runtime.rb"): + with self.subTest(suite=suite, suffix=suffix): + plan = ci.select(["tests/" + suite + suffix]) + self.assertEqual(plan["spinel_tests"], [suite]) + self.assertIn("framework-tests-spinel", plan["jobs"]) + def test_runtime_owners_choose_asymmetric_focused_binaries(self): cases = { "runtime/spinel/web_push_crypto.rb": "spinel_web_push_crypto", @@ -502,18 +515,18 @@ def test_shared_runtime_and_driver_inputs_select_real_harnesses(self): "tests/spinel_stmt_cache_lru.rb": ["spinel_stmt_cache_lru"], "tests/support/db_concurrency_spinel.rb": ["db_sqlite_concurrency"], "runtime/spinel/db.rb": [ - "spinel_db_lease", "param_binds", "spinel_stmt_cache_lru", + "spinel_db_lease", *ci.PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ], "runtime/spinel/sqlite_adapter.rb": [ "date_columns_spinel", "spinel_db_lease", - "param_binds", + *ci.PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ], "runtime/spinel/active_support_time_parsing.rb": [ - "spinel_db_lease", "param_binds", "spinel_stmt_cache_lru", + "spinel_db_lease", *ci.PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ], "runtime/spinel/date.rb": ["date_columns_spinel"], @@ -560,7 +573,7 @@ def test_union_and_deletion_paths_keep_each_owner(self): "date_columns_spinel", "spinel_web_push_crypto", "spinel_db_lease", - "param_binds", + *ci.PARAM_BIND_TESTS, "spinel_stmt_cache_lru", "db_sqlite_concurrency", ], diff --git a/tests/db_bind_bool.rs b/tests/db_bind_bool.rs new file mode 100644 index 000000000..b1232d376 --- /dev/null +++ b/tests/db_bind_bool.rs @@ -0,0 +1,40 @@ +//! JDBC setter contract; the CRuby/Spinel runtime proof lives in param_binds. + +use std::process::Command; + +#[test] +fn jruby_boolean_binding_distinguishes_null_from_false() { + let script = r#" +src = File.read("runtime/spinel/db_jruby.rb") +defn = src[/^ def self\.bind_bool\(stmt, idx, value\)\n.*?^ end\n/m] or abort "no bind_bool" +module Db; end +module Java + module JavaSql + module Types + INTEGER = 4 + end + end +end +Db.module_eval(defn) +class JdbcBoolRecorder + attr_reader :call + def set_null(index, type); @call = [:null, index, type]; end + def set_int(index, value); @call = [:int, index, value]; end +end +handle = Struct.new(:pstmt).new(JdbcBoolRecorder.new) +[[false, [:int, 3, 0]], [nil, [:null, 3, 4]], [true, [:int, 3, 1]], + [nil, [:null, 3, 4]], [false, [:int, 3, 0]]].each do |value, expected| + Db.bind_bool(handle, 3, value) + abort handle.pstmt.call.inspect unless handle.pstmt.call == expected +end +Db.bind_bool(Struct.new(:pstmt).new(nil), 1, nil) +puts "JRuby nullable boolean setter contract passed (no JDBC runtime)" +"#; + let output = Command::new("ruby") + .args(["-e", script]) + .current_dir(env!("CARGO_MANIFEST_DIR")) + .output() + .expect("ruby"); + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + print!("{}", String::from_utf8_lossy(&output.stdout)); +} diff --git a/tests/db_escape_binary.rs b/tests/db_escape_binary.rs index 29007df45..c2e63bc3f 100644 --- a/tests/db_escape_binary.rs +++ b/tests/db_escape_binary.rs @@ -59,3 +59,54 @@ fn the_cruby_shim_writes_bytes_as_a_blob_literal() { fn the_jruby_shim_writes_bytes_as_a_blob_literal() { check("runtime/spinel/db_jruby.rb"); } + +#[test] +fn the_jruby_text_binder_selects_the_writers_storage_class() { + // Exercise the JDBC call boundary without requiring a JDK in the unit + // job. Actual JDBC execution belongs to the JRuby runtime lane. + let script = r##" +src = File.read("runtime/spinel/db_jruby.rb") +defn = src[/^ def self\.bind_text\(stmt, idx, value\)\n.*?^ end\n/m] or abort "no bind_text" +module Db; end +Db.module_eval(defn) +class String + def to_java_bytes; bytes; end +end +class JdbcBindRecorder + attr_reader :call + def set_bytes(index, value); @call = [:blob, index, value]; end + def set_string(index, value); @call = [:text, index, value.bytes]; end +end +Handle = Struct.new(:pstmt) +cases = [ + ["a\0b", :blob], + ["a\0b".b, :blob], + [[255, 254, 39].pack("C*"), :blob], + ["café".b, :blob], + ["abc".b, :text], + ["".b, :text], + ["it's", :text], + ["雪 café", :text], +] +recorder = JdbcBindRecorder.new +handle = Handle.new(recorder) +cases.each do |value, storage| + Db.bind_text(handle, 3, value) + expected = [storage, 3, value.bytes] + abort "#{value.inspect}: #{recorder.call.inspect} != #{expected.inspect}" if recorder.call != expected +end +Db.bind_text(Handle.new(nil), 1, "unused") +print "OK" +"##; + let out = Command::new("ruby") + .args(["-e", script]) + .current_dir(env!("CARGO_MANIFEST_DIR")) + .output() + .expect("ruby"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(String::from_utf8_lossy(&out.stdout), "OK"); +} diff --git a/tests/db_shim_conformance.rs b/tests/db_shim_conformance.rs index 676ad2417..a2898dd1d 100644 --- a/tests/db_shim_conformance.rs +++ b/tests/db_shim_conformance.rs @@ -403,6 +403,17 @@ const RUBY_FAMILY_LIFECYCLE: &[&str] = &[ "checkpoint_in_background!", ]; +#[test] +fn ruby_family_shims_declare_optional_binds() { + let rbs = read_shim("runtime/ruby/db.rbs"); + for method in ["bind_int_opt", "bind_text_opt", "bind_bool_opt"] { + assert!(defines_in_rbs(&rbs, method), "missing RBS: {method}"); + for path in ["runtime/spinel/db.rb", "runtime/spinel/db_cruby.rb", "runtime/spinel/db_jruby.rb"] { + assert!(defines_with(&read_shim(path), method, &["def self."], false, false), "{path}: {method}"); + } + } +} + #[test] fn every_ruby_family_shim_defines_the_request_lifecycle_hooks() { let mut missing: Vec = Vec::new(); diff --git a/tests/param_binds.rs b/tests/param_binds.rs index 5fa71c81c..313950f79 100644 --- a/tests/param_binds.rs +++ b/tests/param_binds.rs @@ -41,10 +41,17 @@ ActiveRecord::Schema[8.1].define(version: 1) do create_table "parents", force: :cascade do |t| t.string "name", null: false t.integer "other_id", null: false + t.integer "number" + t.string "optional_name" + t.boolean "flag" end create_table "items", force: :cascade do |t| t.integer "parent_id", null: false t.string "name", null: false + t.integer "number" + t.string "optional_name" + t.boolean "flag" + t.boolean "required_flag", null: false end end "#, @@ -97,6 +104,36 @@ class Parent < ApplicationRecord row = Item.find_by(name: @name) row.nil? ? -1 : row.id end + def nullable_pair(id, number, name, flag, parent) + @id = id.to_i + @number = number.nil? ? nil : number.to_i + @optional_name = name.nil? ? nil : name.to_s + @flag = flag.nil? ? nil : flag == true + @other_id = parent.to_i + Item.where(id: @id, number: @number, optional_name: @optional_name, flag: @flag, parent_id: @other_id).count + end + def nullable_key(number) + @number = number.nil? ? nil : number.to_i + Item.where(id: @number).count + end + def not_null_number(number) + @number = number.nil? ? nil : number.to_i + Item.where(parent_id: @number).count + end + def not_null_name(name) + @optional_name = name.nil? ? nil : name.to_s + Item.where(name: @optional_name).count + end + def not_null_flag(flag) + @flag = flag.nil? ? nil : flag == true + Item.where(required_flag: @flag).count + end + def nil_key_exists + Item.exists?(nil) + end + def nil_key_find + Item.find(nil) + end end "#, ) @@ -143,11 +180,35 @@ fn emitted(test: &str, target: BuildTarget) { "rows(value)", "count_id(value)", "pair(value, parent)", + "nullable_pair(id, number, name, flag, parent)", "items", ] { assert_bound(&probe, method, "bind_int", binds_on); } assert_bound(&probe, "named(value)", "bind_text", binds_on); + let nullable = probe.split(" def nullable_pair(").nth(1).unwrap(); + let nullable = nullable.split("\n end").next().unwrap(); + for (column, escape) in [ + ("number", "escape_int_opt"), + ("optional_name", "escape_string_opt"), + ("flag", "escape_bool_opt"), + ] { + assert!(nullable.contains(&format!("{column} IS NULL")), "{nullable}"); + assert!(!nullable.contains(&format!("{column} IS ?")), "{nullable}"); + if !binds_on { + assert!(nullable.contains(&format!("Db.{escape}(")), "{nullable}"); + } + } + if binds_on { + assert_eq!(nullable.matches("Db.bind_").count(), 5, "{nullable}"); + assert!(nullable.contains("Db.bind_int("), "{nullable}"); + assert!(nullable.contains("Db.bind_int_opt("), "{nullable}"); + assert!(nullable.contains("Db.bind_text_opt("), "{nullable}"); + assert!(nullable.contains("Db.bind_bool_opt("), "{nullable}"); + let key = probe.split(" def nullable_key(").nth(1).unwrap().split("\n end").next().unwrap(); + assert!(key.contains("WHERE id = ?"), "{key}"); + assert!(key.contains("Db.bind_int_opt(stmt, 1, @number)"), "{key}"); + } let item = std::fs::read_to_string(dir.join("app/models/item.rb")).unwrap(); for method in [ "self._adapter_find_by_id(id)", @@ -246,6 +307,13 @@ fn run_script(dir: &std::path::Path, script: &str, native: bool) { } } +struct ScratchDir(PathBuf); +impl Drop for ScratchDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + fn runtime(native: bool) { let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let base = option_env!("CARGO_TARGET_TMPDIR") @@ -256,6 +324,7 @@ fn runtime(native: bool) { std::process::id() )); std::fs::create_dir_all(&dir).unwrap(); + let _cleanup = ScratchDir(dir.clone()); let prelude = if native { for name in ["db.rb", "active_support_time_parsing.rb"] { std::fs::copy(root.join("runtime/spinel").join(name), dir.join(name)).unwrap(); @@ -342,7 +411,8 @@ puts "runtime: CRuby interrupted reader keeps ownership across a different id" 1, "missing or ambiguous lifecycle probe marker" ); - let body = body.replace(marker, &format!("{lifecycle}\n{marker}")); + let nil_contract = include_str!("param_binds_nil.rb"); + let body = body.replace(marker, &format!("{nil_contract}\n{lifecycle}\n{marker}")); let body = body.replace( "# Observe bytes as a BLOB", &format!("{clear}\n# Observe bytes as a BLOB"), @@ -353,13 +423,27 @@ puts "runtime: CRuby interrupted reader keeps ownership across a different id" } else { include_str!("param_binds_cruby_cache.rb") }; + // Spinel's existing inline writer cannot put NUL in SQL text. Its + // bind-byte contract above still covers NUL; the gem/JDBC writers use + // BLOB literals, so the gem path also checks writer/reader parity. + let nul_writer = if native { + "" + } else { + r#" +inline_bound_string("NUL UTF-8", "a\0b") +inline_bound_string("NUL binary", "a\0b".b) +inline_bound_string("invalid UTF-8 tag", "\xFFa".force_encoding(Encoding::UTF_8)) +inline_bound_string("gem BLOB wrapper", SQLite3::Blob.new("plain-ascii")) +puts "runtime: NUL inline writes and bound reads agree" +"# + }; let script = format!( "{prelude}\nENV[\"DATABASE_POOL_SIZE\"] = \"1\"\n\ Db.configure(\"file:cache_cases?mode=memory&cache=shared\", pool_size: 1)\n\ {cache_cases}\nStatementCacheTest.new.run\n\ puts \"runtime: 12 statement cache ownership and error tests passed\"\nDb.close\n\ ENV[\"DATABASE_POOL_SIZE\"] = \"4\"\n\ - Db.configure(\"file:bind_runtime?mode=memory&cache=shared\", pool_size: 4)\n{body}\n{ownership}\nDb.close\n" + Db.configure(\"file:bind_runtime?mode=memory&cache=shared\", pool_size: 4)\n{body}\n{nul_writer}\n{ownership}\nDb.close\n" ); run_script(&dir, &script, native); std::fs::remove_dir_all(dir).expect("remove successful runtime probe"); @@ -387,6 +471,267 @@ fn bind_runtime_spinel() { runtime(true); } +// A String-only override exposes the inherited Integer adapter seed that +// SQLite's escape helpers otherwise hide. Compile the actual emitted RBS; +// an unseeded native compile does not exercise this boundary. +fn string_key_adapter(test: &str, target: BuildTarget) { + if std::env::var_os("ROUNDHOUSE_BINDS_CHILD").is_none() { + for mode in ["0", "1"] { + println!("{test}: ROUNDHOUSE_PARAM_BINDS={mode}"); + success( + Command::new(std::env::current_exe().unwrap()) + .args(["--exact", test, "--include-ignored", "--nocapture"]) + .env("ROUNDHOUSE_BINDS_CHILD", "1") + .env("ROUNDHOUSE_PARAM_BINDS", mode), + ); + } + return; + } + let (dir, errors) = emit_and_run::empty_app() + .write( + "app/models/application_record.rb", + "class ApplicationRecord < ActiveRecord::Base\n self.abstract_class = true\nend\n", + ) + .write( + "app/models/widget.rb", + "class Widget < ApplicationRecord\n self.primary_key = \"identifier\"\nend\n", + ) + .write( + "app/controllers/application_controller.rb", + "class ApplicationController < ActionController::Base\nend\n", + ) + .write("config/routes.rb", "Rails.application.routes.draw do\nend\n") + .write( + "db/schema.rb", + "ActiveRecord::Schema[8.1].define(version: 1) do\n create_table \"widgets\", primary_key: \"identifier\", id: :string do |t|\n t.string \"name\"\n end\nend\n", + ) + .emit(target); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let _cleanup = ScratchDir(dir.parent().unwrap().to_path_buf()); + let script = r#"require_relative "boot" +class Widget + def self._adapter_find_by_id(id) + record = Widget.new + record.name = id.upcase + record + end + def self._adapter_exists_by_id?(id) + id.upcase == "LITERAL_KEY" + end +end +raise "String adapter input" unless Widget.find("literal_key").name == "LITERAL_KEY" +raise "empty String key" unless Widget.find("").name == "" +raise "zero String key" unless Widget.find("0").name == "0" +raise "String exists adapter" unless Widget.exists?("literal_key") +raise "missing String exists adapter" if Widget.exists?("other") +raise "nil exists guard" if Widget.exists?(nil) +begin + Widget.find(nil) + raise "nil reached adapter" +rescue ActiveRecord::RecordNotFound +end +puts "PASS seeded String-only find/exists adapter boundary and nil guards" +"#; + if target == BuildTarget::Spinel { + std::fs::write(dir.join("strict_adapter.rb"), script).unwrap(); + let compiler = std::env::var("SPINEL").unwrap_or_else(|_| "spinel".into()); + let mut command = Command::new(compiler); + command + .args(["--rbs", ".", "strict_adapter.rb", "-o", "strict_adapter"]) + .current_dir(&dir); + let output = command.output().expect("compile seeded String adapter"); + assert!( + !String::from_utf8_lossy(&output.stderr).contains("type seeds are unavailable"), + "RBS extractor is required: {}", + String::from_utf8_lossy(&output.stderr) + ); + check_success(&command, &output); + success(Command::new(dir.join("strict_adapter")).current_dir(&dir)); + } else { + run_script(&dir, script, false); + } +} + +#[test] +fn string_key_adapter_ruby() { + string_key_adapter("string_key_adapter_ruby", BuildTarget::Ruby); +} + +#[test] +#[ignore = "requires Spinel and its RBS extractor"] +fn string_key_adapter_spinel() { + string_key_adapter("string_key_adapter_spinel", BuildTarget::Spinel); +} + +fn nullable_associations(test: &str, target: BuildTarget) { + if std::env::var_os("ROUNDHOUSE_BINDS_CHILD").is_none() { + for mode in ["0", "1"] { + println!("{test}: ROUNDHOUSE_PARAM_BINDS={mode}"); + success( + Command::new(std::env::current_exe().unwrap()) + .args(["--exact", test, "--include-ignored", "--nocapture"]) + .env("ROUNDHOUSE_BINDS_CHILD", "1") + .env("ROUNDHOUSE_PARAM_BINDS", mode), + ); + } + return; + } + let app = emit_and_run::empty_app() + .write( + "app/models/application_record.rb", + "class ApplicationRecord < ActiveRecord::Base\n self.abstract_class = true\nend\n", + ) + .write( + "app/controllers/application_controller.rb", + "class ApplicationController < ActionController::Base\nend\n", + ) + .write( + "config/routes.rb", + "Rails.application.routes.draw do\nend\n", + ) + .write( + "app/models/account.rb", + "class Account < ApplicationRecord\n has_many :links\n has_many :taggings, as: :taggable\nend\n", + ) + .write( + "app/models/article.rb", + "class Article < ApplicationRecord\n has_many :links\nend\n", + ) + .write( + "app/models/link.rb", + "class Link < ApplicationRecord\n belongs_to :account, optional: true\n belongs_to :article, optional: true\nend\n", + ) + .write( + "app/models/tagging.rb", + "class Tagging < ApplicationRecord\n belongs_to :taggable, polymorphic: true, optional: true\nend\n", + ) + .write( + "db/schema.rb", + r#" +ActiveRecord::Schema[8.1].define(version: 1) do + create_table "accounts", force: :cascade do |t| + t.string "name", null: false + end + create_table "articles", id: :uuid, force: :cascade do |t| + t.string "title", null: false + end + create_table "links", force: :cascade do |t| + t.integer "account_id" + t.uuid "article_id" + end + create_table "taggings", force: :cascade do |t| + t.integer "taggable_id" + t.string "taggable_type" + end +end +"#, + ); + let (dir, errors) = app.emit(target); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let binds_on = std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1"; + let link = std::fs::read_to_string(dir.join("app/models/link.rb")).unwrap(); + assert_bound(&link, "account", "bind_int", binds_on); + assert_bound(&link, "article", "bind_text", binds_on); + let tagging = std::fs::read_to_string(dir.join("app/models/tagging.rb")).unwrap(); + assert_bound(&tagging, "taggable", "bind_int", binds_on); + // Spinel currently boxes column_int_opt's nil sentinel as an Integer + // when passing it to a generated setter. That separate hydration gap + // makes .nil? false before this reader runs. Exercise native integer + // nil inputs through the post-emission assignments in the shared + // script, and retain native SQL-NULL hydration coverage for UUIDs. + let integer_hydration = if target == BuildTarget::Spinel { + "" + } else { + r#" +raise "nil integer FK from SQL resolved an association" unless from_sql.account.nil? +raise "nil polymorphic FK from SQL resolved an association" unless Tagging.find(1).taggable.nil? +raise "nil polymorphic type from SQL resolved an association" unless Tagging.find(3).taggable.nil? +puts "emit: SQL-NULL integer and polymorphic association readers passed" +"# + }; + let script = format!( + r#"require_relative "boot" +require_relative "app/models/account" +require_relative "app/models/article" +require_relative "app/models/link" +require_relative "app/models/tagging" +SqliteAdapter.configure("file:nil_associations?mode=memory&cache=shared") +ActiveRecord.adapter = SqliteAdapter +Schema.statements.each {{ |sql| Db.exec(sql) }} +{} +{} +from_sql = Link.find(1) +raise "nil UUID FK from SQL resolved an association" unless from_sql.article.nil? +puts "emit: SQL-NULL UUID association reader passed" +{integer_hydration} +Db.close +"#, + cache_probe(target == BuildTarget::Spinel), + include_str!("param_binds_associations.rb") + ); + run_script(&dir, &script, target == BuildTarget::Spinel); +} + +#[test] +fn nullable_associations_ruby() { + nullable_associations("nullable_associations_ruby", BuildTarget::Ruby); +} + +#[test] +#[ignore = "requires Spinel (SPINEL=/path/to/spinel)"] +fn nullable_associations_spinel() { + nullable_associations("nullable_associations_spinel", BuildTarget::Spinel); +} + +fn cache_probe(native: bool) -> &'static str { + if native { + r#" +module SQL + ffi_func :sqlite3_next_stmt, [:ptr, :ptr], :ptr +end +class DbConn + def gate_cache_size + @entries.length + end + def gate_live_statements + n = 0 + ptr = SQL.sqlite3_next_stmt(dbh, nil) + while !ptr.nil? + n += 1 + ptr = SQL.sqlite3_next_stmt(dbh, ptr) + end + n + end +end +module Db + def self.gate_cache_size + current_conn.gate_cache_size + end + def self.gate_live_statements + current_conn.gate_live_statements + end + def self.gate_released(stmt) + nil + end +end +"# + } else { + r#" +module Db + def self.gate_cache_size + (current_dbh.instance_variable_get(:@rh_stmt_cache) || {}).size + end + def self.gate_live_statements + 0 + end + def self.gate_released(stmt) + raise "transient statement was not closed" if stmt[:stmt] && !stmt[:stmt].closed? + end +end +"# + } +} + fn raw_where_substitution(target: BuildTarget) { let (dir, errors) = overlay().emit(target); assert!(errors.is_empty(), "{}", errors.join("\n")); diff --git a/tests/param_binds_associations.rb b/tests/param_binds_associations.rb new file mode 100644 index 000000000..9caa4f225 --- /dev/null +++ b/tests/param_binds_associations.rb @@ -0,0 +1,35 @@ +# Real sentinel-key rows distinguish nil from accidental 0/"" coercion. +# The Rails source has no explicit nil assignment to supply inferred +# nilability to a synthesized association's Arel RHS. These assignments +# happen after emission, so only the reader's own guard can protect it. +Db.exec("INSERT INTO accounts (id, name) VALUES (0, 'zero trap'), (7, 'present')") +Db.exec("INSERT INTO articles (id, title) VALUES ('', 'empty trap'), ('00000000-0000-0000-0000-000000000007', 'present')") +Db.exec("INSERT INTO links (id, account_id, article_id) VALUES (1, NULL, NULL), (2, 7, '00000000-0000-0000-0000-000000000007')") +Db.exec("INSERT INTO taggings (id, taggable_id, taggable_type) VALUES (1, NULL, 'Account'), (2, 7, 'Account'), (3, NULL, NULL)") + +8.times do + absent = Link.new + absent.account_id = nil + absent.article_id = nil + untagged = Tagging.new + untagged.taggable_id = nil + untagged.taggable_type = "Account" + cache_before = Db.gate_cache_size + raise "nil integer FK resolved an association" unless absent.account.nil? + raise "nil UUID FK resolved an association" unless absent.article.nil? + raise "nil polymorphic FK resolved an association" unless untagged.taggable.nil? + raise "nil FK prepared a query" unless Db.gate_cache_size == cache_before + present = Link.find(2) + account = present.account + article = present.article + raise "present integer FK lost" if account.nil? + raise "present integer FK changed" unless account.id == 7 + raise "present UUID FK lost" if article.nil? + raise "present UUID FK changed" unless article.id == "00000000-0000-0000-0000-000000000007" + tagged = Tagging.find(2).taggable + raise "present polymorphic FK lost" if tagged.nil? + raise "present polymorphic FK changed" unless tagged.id == 7 + untagged.taggable_type = nil + raise "nil polymorphic type resolved an association" unless untagged.taggable.nil? +end +puts "emit: explicit nil integer/UUID/polymorphic association readers and nonnil controls passed" diff --git a/tests/param_binds_cleanup.rb b/tests/param_binds_cleanup.rb new file mode 100644 index 000000000..956ce005d --- /dev/null +++ b/tests/param_binds_cleanup.rb @@ -0,0 +1,54 @@ +# Faults run inside the actual emitted serialization/bind/hydration methods. +# Every ownership assertion is inside the same lease as the rescued failure. +module CleanupFaults + @current = "" + def self.current=(value) + @current = value + end + def self.trip(value) + raise "injected cleanup " + value if value == @current + nil + end +end + +def expect_cleanup_failure(fault) + CleanupFaults.current = fault + begin + yield + raise "missing cleanup failure " + fault + rescue RuntimeError => error + raise error unless error.message.include?("injected cleanup " + fault) + ensure + CleanupFaults.current = "" + end + count = Db.cleanup_owned_count + raise fault + " left " + count.to_s + " owned statements inside the lease" unless count == 0 + nil +end + +SqliteAdapter.configure("file:cleanup_gate?mode=memory&cache=shared") +ActiveRecord.adapter = SqliteAdapter +Schema.statements.each { |sql| Db.exec(sql) } +Db.exec("INSERT INTO parents (id, name) VALUES (1, 'one')") +Db.exec("INSERT INTO readings (id, parent_id, recorded_at, label) VALUES (1, 1, '2023-11-14 22:13:20.000000', 'one')") +needle = Time.at(1700000000).utc +row = Reading.find(1) +controller = ParentsController.new +binds = ENV["ROUNDHOUSE_PARAM_BINDS"] == "1" +Db.with_connection do + 3.times do + expect_cleanup_failure("serialize") { row.time_count(needle) } + expect_cleanup_failure("bind") { row.time_count(needle) } if binds + expect_cleanup_failure("step") { row.time_count(needle) } + expect_cleanup_failure("hydrate") { row.single(1) } + expect_cleanup_failure("hydrate") { row.many(1) } + expect_cleanup_failure("reload") { row._adapter_reload } + expect_cleanup_failure("hydrate") { controller.preloaded } + expect_cleanup_failure("hydrate") { Reading._hydrate_all("SELECT id, parent_id, recorded_at, label FROM readings") } + raise "lease no longer usable" unless row.time_count(needle) == 1 + raise "successful recovery retained a statement" unless Db.cleanup_owned_count == 0 + end + true +end +puts "cleanup: " + (binds ? "24" : "21") + " rescued generated failures; every owned count was zero before lease end" +Db.close diff --git a/tests/param_binds_cleanup.rs b/tests/param_binds_cleanup.rs new file mode 100644 index 000000000..eb5f74365 --- /dev/null +++ b/tests/param_binds_cleanup.rs @@ -0,0 +1,125 @@ +//! Generated reads must release statements before a rescued failure returns +//! to an ongoing lease, including failures before the driver's bind method. + +#[path = "support/emit_and_run.rs"] +mod emit_and_run; + +use roundhouse::project::BuildTarget; +use std::path::Path; +use std::process::Command; + +fn success(command: &mut Command) { + let output = command.output().unwrap_or_else(|e| panic!("{command:?}: {e}")); + assert!(output.status.success(), "{command:?}: {}\n{}\n{}", output.status, + String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr)); + print!("{}", String::from_utf8_lossy(&output.stdout)); +} + +fn inject(path: &Path, header: &str, fault: &str) { + let source = std::fs::read_to_string(path).unwrap(); + assert_eq!(source.matches(header).count(), 1, "{}: {header}", path.display()); + std::fs::write(path, source.replace(header, + &format!("{header}\n CleanupFaults.trip(\"{fault}\")"))).unwrap(); +} + +fn emitted(test: &str, target: BuildTarget) { + if std::env::var_os("ROUNDHOUSE_BINDS_CHILD").is_none() { + for mode in ["0", "1"] { + success(Command::new(std::env::current_exe().unwrap()) + .args(["--exact", test, "--include-ignored", "--nocapture"]) + .env("ROUNDHOUSE_BINDS_CHILD", "1").env("ROUNDHOUSE_PARAM_BINDS", mode)); + } + return; + } + let (dir, errors) = emit_and_run::empty_app() + .write("app/models/application_record.rb", "class ApplicationRecord < ActiveRecord::Base\n self.abstract_class = true\nend\n") + .write("app/controllers/application_controller.rb", "class ApplicationController < ActionController::Base\nend\n") + .write("app/controllers/parents_controller.rb", "class ParentsController < ApplicationController\n def index\n render plain: preloaded.length.to_s\n end\n def preloaded\n Parent.includes(:readings).to_a\n end\nend\n") + .write("config/routes.rb", "Rails.application.routes.draw do\n get '/parents', to: 'parents#index'\nend\n") + .write("sig/reading.rbs", "class Reading\n def time_count: (Time value) -> Integer\nend\n") + .write("db/schema.rb", r#" +ActiveRecord::Schema[8.1].define(version: 1) do + create_table :parents do |t| + t.string :name, null: false + end + create_table :readings do |t| + t.integer :parent_id, null: false + t.datetime :recorded_at, null: false + t.string :label, null: false + end +end +"#) + .write("app/models/parent.rb", r#" +class Parent < ApplicationRecord + has_many :readings +end +"#) + .write("app/models/reading.rb", r#" +class Reading < ApplicationRecord + belongs_to :parent + def time_count(value) + @needle = value + Reading.where(recorded_at: @needle).count + end + def single(value) + @id = value.to_i + Reading.find_by(id: @id) + end + def many(value) + @id = value.to_i + Reading.where(id: @id).to_a + end +end +"#).emit(target); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let path = dir.join("app/models/reading.rb"); + let source = std::fs::read_to_string(&path).unwrap(); + for method in ["time_count(value)", "single(value)", "many(value)", "_adapter_reload"] { + let body = source.split_once(&format!(" def {method}\n")).unwrap().1 + .split_once("\n end").unwrap().0; + assert!(body.contains("\n ensure\n Db.finalize(stmt)"), "{method}: {body}"); + } + let parent = std::fs::read_to_string(dir.join("app/controllers/parents_controller.rb")).unwrap(); + assert!(parent.contains("ensure\n Db.finalize(__readings_stmt)"), "{parent}"); + inject(&path, " def self.from_stmt(stmt)", "hydrate"); + inject(&dir.join("runtime/active_support_time_parsing.rb"), " def self.format_db_time(value)", "serialize"); + let native = target == BuildTarget::Spinel; + inject(&dir.join("runtime/db.rb"), if native { " def self.bind_text(stmt, idx, value)" } else { " def self.bind_text(handle, idx, value)" }, "bind"); + inject(&dir.join("runtime/db.rb"), if native { " def self.step?(stmt)" } else { " def self.step?(handle)\n entry = handle" }, "step"); + inject(&dir.join("runtime/db.rb"), if native { " def self.column_int(stmt, i)" } else { " def self.column_int(handle, i)" }, "reload"); + let probe = if native { + "class DbConn\n def cleanup_owned_count\n @open.length\n end\nend\nmodule Db\n def self.cleanup_owned_count\n current_conn.cleanup_owned_count\n end\nend\n" + } else { + "module Db\n def self.cleanup_owned_count\n open_statements(current_dbh).size\n end\nend\n" + }; + let script = format!("require_relative \"boot\"\nrequire_relative \"app/models/parent\"\nrequire_relative \"app/models/reading\"\nrequire_relative \"app/controllers/parents_controller\"\n{probe}\n{}", include_str!("param_binds_cleanup.rb")); + std::fs::write(dir.join("cleanup_gate.rb"), script).unwrap(); + println!("cleanup tree: {} binds={}", dir.display(), std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap()); + if native { + success(Command::new(std::env::var("SPINEL").unwrap_or_else(|_| "spinel".into())) + .args(["cleanup_gate.rb", "-o", "cleanup_gate"]).current_dir(&*dir)); + success(Command::new(dir.join("cleanup_gate")).current_dir(&*dir)); + } else { + success(emit_and_run::ruby().arg("cleanup_gate.rb").current_dir(&*dir)); + } +} + +#[test] +fn generated_cleanup_ruby() { + emitted("generated_cleanup_ruby", BuildTarget::Ruby); +} + +#[test] +#[ignore = "requires Spinel (SPINEL=/path/to/spinel)"] +fn generated_cleanup_spinel() { + emitted("generated_cleanup_spinel", BuildTarget::Spinel); +} + +#[test] +fn text_preprocessing_cleanup_ruby() { + success(emit_and_run::ruby().arg("-r") + .arg(Path::new(env!("CARGO_MANIFEST_DIR")).join("runtime/ruby/active_record/connection_pool.rb")) + .arg("-r") + .arg(Path::new(env!("CARGO_MANIFEST_DIR")).join("runtime/spinel/db_cruby.rb")) + .arg(Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/param_binds_text_cleanup.rb"))); +} diff --git a/tests/param_binds_emit.rb b/tests/param_binds_emit.rb index f64535da8..81e220db5 100644 --- a/tests/param_binds_emit.rb +++ b/tests/param_binds_emit.rb @@ -6,12 +6,12 @@ Db.exec("INSERT INTO parents (id, other_id, name) VALUES (" + i.to_s + ", 0, 'parent-" + i.to_s + "')") # Deliberately asymmetric: swapping (id, parent_id) must not find a # different row with the same COUNT(*). A reversal permutation hides it! - Db.exec("INSERT INTO items (id, parent_id, name) VALUES (" + i.to_s + ", " + ((i % 32) + 1).to_s + ", 'row-" + i.to_s + "')") + Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (" + i.to_s + ", " + ((i % 32) + 1).to_s + ", 'row-" + i.to_s + "', 0)") # Distinct group sizes make a count expose stale *existing* ids too. j = 0 while j < i % 4 extra_id = 1000 + i * 10 + j - Db.exec("INSERT INTO items (id, parent_id, name) VALUES (" + extra_id.to_s + ", " + i.to_s + ", 'extra-" + extra_id.to_s + "')") + Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (" + extra_id.to_s + ", " + i.to_s + ", 'extra-" + extra_id.to_s + "', 0)") j += 1 end i += 1 @@ -48,3 +48,66 @@ Db.query_cache_end end puts "emit: 32 ids, 96 serial interleaved rounds, nine read methods passed" + +# Nullable values must select NULL rows in either emit mode. The integer +# predicates on either side pin the order of the three optional bind slots. +Db.exec("INSERT INTO items (id, parent_id, name, number, optional_name, flag, required_flag) VALUES (501, 500, 'nullable', NULL, NULL, NULL, 0)") +Db.exec("INSERT INTO items (id, parent_id, name, number, optional_name, flag, required_flag) VALUES (502, 500, 'nullable', 0, '', 0, 0)") +Db.exec("INSERT INTO items (id, parent_id, name, number, optional_name, flag, required_flag) VALUES (503, 500, 'nullable', 73, '雪', 1, 0)") +Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (0, 500, 'zero-key', 0)") +Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (504, 0, 'zero-parent', 0)") +Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (505, 500, '', 1)") +i = 0 +while i < 12 + raise "nullable nil row" unless probe.nullable_pair(501, nil, nil, nil, 500) == 1 + raise "nullable zero row" unless probe.nullable_pair(502, 0, "", false, 500) == 1 + raise "nullable value row" unless probe.nullable_pair(503, 73, "雪", true, 500) == 1 + raise "nil must not match false/zero/empty" unless probe.nullable_pair(502, nil, nil, nil, 500) == 0 + raise "nullable final bind position" unless probe.nullable_pair(501, nil, nil, nil, 501) == 0 + raise "nullable RHS against nonnullable key" unless probe.nullable_key(nil) == 0 + raise "nil matched a NOT NULL zero value" unless probe.not_null_number(nil) == 0 + raise "NOT NULL zero remains a real value" unless probe.not_null_number(0) == 1 + raise "zero key is a real value" unless probe.nullable_key(0) == 1 + raise "nil matched a NOT NULL empty string" unless probe.not_null_name(nil) == 0 + raise "NOT NULL empty string remains a real value" unless probe.not_null_name("") == 1 + raise "nil matched a NOT NULL boolean" unless probe.not_null_flag(nil) == 0 + raise "NOT NULL false remains a real value" unless probe.not_null_flag(false) == 85 + raise "NOT NULL true remains a real value" unless probe.not_null_flag(true) == 1 + i += 1 +end +puts "emit: nullable int/text/bool and surrounding bind positions passed" + +# Three independently nullable predicates have eight SQL shapes. Visit +# every combination repeatedly with a different id and a fixed trailing +# predicate; a skipped NULL slot must not shift any later bind incorrectly. +mask = 0 +while mask < 8 + number_sql = (mask & 1) == 0 ? "NULL" : "73" + name_sql = (mask & 2) == 0 ? "NULL" : "'snow'" + flag_sql = (mask & 4) == 0 ? "NULL" : "1" + Db.exec("INSERT INTO items (id, parent_id, name, number, optional_name, flag, required_flag) VALUES (" + + (600 + mask).to_s + ", 900, 'combination', " + number_sql + ", " + name_sql + ", " + flag_sql + ", 0)") + mask += 1 +end +12.times do + mask = 0 + while mask < 8 + number = (mask & 1) == 0 ? nil : 73 + name = (mask & 2) == 0 ? nil : "snow" + flag = (mask & 4) == 0 ? nil : true + raise "nullable combination #{mask}" unless probe.nullable_pair(600 + mask, number, name, flag, 900) == 1 + raise "nullable combination trailing bind #{mask}" unless probe.nullable_pair(600 + mask, number, name, flag, 901) == 0 + mask += 1 + end +end +puts "emit: all eight nullable SQL shapes, 192 interleaved hit/miss reads passed" + +# Public key methods receive nil even though their synthesized primitive is +# key-typed. Rails rejects it before touching SQLite; id=0 must not match. +raise "exists?(nil) matched zero key" if probe.nil_key_exists +begin + probe.nil_key_find + raise "find(nil) matched zero key" +rescue ActiveRecord::RecordNotFound +end +puts "emit: find(nil) and exists?(nil) reject a real zero key passed" diff --git a/tests/param_binds_nil.rb b/tests/param_binds_nil.rb new file mode 100644 index 000000000..7a03e3c31 --- /dev/null +++ b/tests/param_binds_nil.rb @@ -0,0 +1,20 @@ +# Contract item 8: nil against a NOT NULL column must never select zero. +# Use the typed primitive directly, as well as the emitted optional path. +Db.exec("CREATE TABLE bind_not_null (value INTEGER NOT NULL)") +Db.exec("INSERT INTO bind_not_null VALUES (0), (73)") +def check_not_null_integer(value) + stmt = Db.prepare("SELECT COUNT(*) FROM bind_not_null WHERE value = ?") + Db.bind_int(stmt, 1, value) + raise "missing NOT NULL count" unless Db.step?(stmt) + expected = value.nil? ? 0 : 1 + actual = Db.column_int(stmt, 0) + Db.finalize(stmt) + raise "typed bind_int(nil) matched zero" unless actual == expected +end +12.times do + check_not_null_integer(73) + check_not_null_integer(nil) + check_not_null_integer(0) + check_not_null_integer(nil) +end +puts "runtime: 48 typed integer/NULL comparisons against NOT NULL passed" diff --git a/tests/param_binds_planner.rb b/tests/param_binds_planner.rb new file mode 100644 index 000000000..b863dcf6e --- /dev/null +++ b/tests/param_binds_planner.rb @@ -0,0 +1,124 @@ +require_relative "boot" +require_relative "app/models/indexed_row" +require_relative "app/models/paired_row" +require_relative "app/models/joined_row" +require_relative "app/models/wide_row" +SqliteAdapter.configure(":memory:") +ActiveRecord.adapter = SqliteAdapter +Schema.statements.each { |sql| Db.exec(sql) } + +# The Arel fast path does not yet fold joins. A view lets the emitted +# nullable WHERE predicate participate in SQLite's real LEFT JOIN planner +# without adding unsupported joins to the lowerer or rewriting emitted SQL. +Db.exec("DROP TABLE joined_rows") +Db.exec("CREATE VIEW joined_rows AS SELECT host_rows.id, indexed_rows.a FROM host_rows LEFT JOIN indexed_rows ON indexed_rows.id = host_rows.id") +Db.exec("WITH RECURSIVE n(id) AS (SELECT 1 UNION ALL SELECT id + 1 FROM n WHERE id < 1000) INSERT INTO indexed_rows SELECT id, CASE WHEN id = 7 THEN 7 END FROM n") +Db.exec("WITH RECURSIVE n(id) AS (SELECT 1 UNION ALL SELECT id + 1 FROM n WHERE id < 1000) INSERT INTO paired_rows SELECT id, CASE WHEN id = 7 THEN 7 END, CASE WHEN id = 7 THEN 11 END FROM n") +Db.exec("INSERT INTO host_rows SELECT id FROM indexed_rows") +Db.exec("INSERT INTO host_rows VALUES (1001)") +Db.exec("INSERT INTO wide_rows DEFAULT VALUES") +Db.exec("CREATE INDEX single_present ON indexed_rows(a) WHERE a IS NOT NULL") +Db.exec("CREATE INDEX pair_present ON paired_rows(a, b) WHERE a IS NOT NULL AND b IS NOT NULL") +Db.exec("ANALYZE") + +module Db + class << self + attr_reader :planner_sql, :planner_binds + alias planner_original_prepare prepare + alias planner_original_prepare_uncached prepare_uncached + alias planner_original_bind_value bind_value + def prepare(sql) + @planner_sql = sql + @planner_binds = [] + planner_original_prepare(sql) + end + def prepare_uncached(sql) + @planner_sql = sql + @planner_binds = [] + planner_original_prepare_uncached(sql) + end + def bind_value(handle, index, value) + @planner_binds[index - 1] = value + planner_original_bind_value(handle, index, value) + end + def planner_details + current_dbh.execute("EXPLAIN QUERY PLAN " + @planner_sql, @planner_binds).map { |row| row[3] }.join("; ") + end + end +end + +def expect_result(label, expected, actual) + raise "#{label}: expected #{expected}, got #{actual}" unless expected == actual +end + +bound = ENV.fetch("PLANNER_BINDS") == "1" +single = IndexedRow.new +pair = PairedRow.new +joined = JoinedRow.new +Db.with_connection do + single_shapes = [] + pair_shapes = [] + 3.times do + expect_result("single non-nil", 1, single.matching(7)) + single_shapes << Db.planner_sql + plan = Db.planner_details + raise "single partial index lost: #{Db.planner_sql}: #{plan}" unless plan.include?("USING COVERING INDEX single_present (a=?)") + expect_result("single placeholder values", bound ? [7] : [], Db.planner_binds) + puts "planner binds=#{bound}: single partial index: #{plan}" + + expect_result("single nil", 999, single.matching(nil)) + single_shapes << Db.planner_sql + raise "nil must remove bind slot: #{Db.planner_sql}" unless Db.planner_sql.include?("a IS NULL") && !Db.planner_sql.include?("?") + expect_result("single nil bind count", [], Db.planner_binds) + + expect_result("pair non-nil", 1, pair.matching(7, 11)) + pair_shapes << Db.planner_sql + plan = Db.planner_details + raise "composite partial index lost: #{Db.planner_sql}: #{plan}" unless plan.include?("USING COVERING INDEX pair_present (a=? AND b=?)") + expect_result("pair placeholder order", bound ? [7, 11] : [], Db.planner_binds) + puts "planner binds=#{bound}: composite partial index: #{plan}" + + expect_result("pair nil/non-nil", 0, pair.matching(nil, 11)) + pair_shapes << Db.planner_sql + expect_result("pair removed first slot", bound ? [11] : [], Db.planner_binds) + expect_result("pair non-nil/nil", 0, pair.matching(7, nil)) + pair_shapes << Db.planner_sql + expect_result("pair removed last slot", bound ? [7] : [], Db.planner_binds) + expect_result("pair nil/nil", 999, pair.matching(nil, nil)) + pair_shapes << Db.planner_sql + expect_result("pair no slots", [], Db.planner_binds) + + expect_result("joined non-nil", 1, joined.matching(7)) + plan = Db.planner_details + raise "LEFT JOIN was not strength-reduced: #{Db.planner_sql}: #{plan}" if plan.include?("LEFT-JOIN") + raise "reduced join lost partial index: #{plan}" unless plan.include?("single_present") + puts "planner binds=#{bound}: LEFT JOIN strength reduction: #{plan}" + + expect_result("joined nil including unmatched row", 1000, joined.matching(nil)) + plan = Db.planner_details + raise "nil must retain LEFT JOIN: #{plan}" unless plan.include?("LEFT-JOIN") + expect_result("join nil bind count", [], Db.planner_binds) + end + expect_result("single shape count", 2, single_shapes.uniq.length) + expect_result("pair shape count", 4, pair_shapes.uniq.length) + # Changing the non-nil values must reuse the already observed bound + # shape. Inline mode intentionally retains value-bearing main SQL. + [8, 19, 1023].each do |value| + expect_result("single changed bind #{value}", 0, single.matching(value)) + expect_result("single non-nil values share one shape", single_shapes[0], Db.planner_sql) if bound + expect_result("pair changed bind #{value}", 0, pair.matching(value, value + 1)) + expect_result("pair non-nil values share one shape", pair_shapes[0], Db.planner_sql) if bound + end + + wide = WideRow.new + cache = Db.current_dbh.instance_variable_get(:@rh_stmt_cache) + cached_before = cache.length + 256.times do |mask| + values = 8.times.map { |bit| (mask & (1 << bit)) == 0 ? nil : bit + 1 } + expect_result("eight-nullable shape #{mask}", mask == 0 ? 1 : 0, wide.matching(*values)) + expect_result("eight-nullable bind order #{mask}", bound ? values.compact : [], Db.planner_binds) + end + expect_result("eight nullable predicates bypass statement cache", cached_before, cache.length) if bound +end +puts "planner: binds=#{bound}, 3 repeated single/composite/join plan groups, 2/4 null patterns, varying non-nil values, 256 eight-nullable combinations passed" +Db.close diff --git a/tests/param_binds_planner.rs b/tests/param_binds_planner.rs new file mode 100644 index 000000000..3d63e0126 --- /dev/null +++ b/tests/param_binds_planner.rs @@ -0,0 +1,117 @@ +//! Runtime nullable shapes must preserve SQLite's partial-index eligibility +//! and outer-join strength reduction. Plans are captured from emitted reads, +//! not independently reconstructed predicates. Each flag gets a subprocess. + +#[path = "support/emit_and_run.rs"] +mod emit_and_run; + +use roundhouse::project::BuildTarget; +use std::process::Command; + +#[test] +fn nullable_read_plans_ruby() { + if std::env::var_os("ROUNDHOUSE_BINDS_CHILD").is_none() { + for mode in ["0", "1"] { + let output = Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "nullable_read_plans_ruby", "--nocapture"]) + .env("ROUNDHOUSE_BINDS_CHILD", "1") + .env("ROUNDHOUSE_PARAM_BINDS", mode) + .output().unwrap(); + assert!(output.status.success(), "binds={mode}\n{}\n{}", + String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr)); + print!("{}", String::from_utf8_lossy(&output.stdout)); + } + return; + } + let (dir, errors) = emit_and_run::empty_app() + .write("app/models/application_record.rb", "class ApplicationRecord < ActiveRecord::Base\n self.abstract_class = true\nend\n") + .write("app/controllers/application_controller.rb", "class ApplicationController < ActionController::Base\nend\n") + .write("config/routes.rb", "Rails.application.routes.draw do\nend\n") + .write("db/schema.rb", r#" +ActiveRecord::Schema[8.1].define(version: 1) do + create_table :indexed_rows do |t| + t.integer :a + end + create_table :paired_rows do |t| + t.integer :a + t.integer :b + end + create_table :host_rows do |t| + end + create_table :joined_rows do |t| + t.integer :a + end + create_table :wide_rows do |t| + t.integer :a + t.integer :b + t.integer :c + t.integer :d + t.integer :e + t.integer :f + t.integer :g + t.integer :h + end +end +"#) + .write("app/models/indexed_row.rb", r#" +class IndexedRow < ApplicationRecord + def matching(value) + @a = value.nil? ? nil : value.to_i + IndexedRow.where(a: @a).count + end +end +"#) + .write("app/models/paired_row.rb", r#" +class PairedRow < ApplicationRecord + def matching(first, second) + @a = first.nil? ? nil : first.to_i + @b = second.nil? ? nil : second.to_i + PairedRow.where(a: @a, b: @b).count + end +end +"#) + .write("app/models/joined_row.rb", r#" +class JoinedRow < ApplicationRecord + def matching(value) + @a = value.nil? ? nil : value.to_i + JoinedRow.where(a: @a).count + end +end +"#) + .write("app/models/wide_row.rb", r#" +class WideRow < ApplicationRecord + def matching(a, b, c, d, e, f, g, h) + @a = a.nil? ? nil : a.to_i + @b = b.nil? ? nil : b.to_i + @c = c.nil? ? nil : c.to_i + @d = d.nil? ? nil : d.to_i + @e = e.nil? ? nil : e.to_i + @f = f.nil? ? nil : f.to_i + @g = g.nil? ? nil : g.to_i + @h = h.nil? ? nil : h.to_i + WideRow.where(a: @a, b: @b, c: @c, d: @d, e: @e, f: @f, g: @g, h: @h).count + end +end +"#) + .emit(BuildTarget::Ruby); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let bound = std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1"; + for model in ["indexed_row", "paired_row", "joined_row", "wide_row"] { + let source = std::fs::read_to_string(dir.join(format!("app/models/{model}.rb"))).unwrap(); + let method = source.split_once(" def matching(").unwrap().1.split_once("\n end").unwrap().0; + assert!(!method.contains(".where("), "read was not lowered: {method}"); + assert_eq!(method.contains("Db.bind_"), bound, "{method}"); + if model == "wide_row" && bound { + assert!(method.contains("Db.prepare_uncached("), "eight nullable predicates must bypass the 128-shape cache: {method}"); + } else { + assert!(method.contains("Db.prepare("), "{method}"); + } + } + std::fs::write(dir.join("planner_gate.rb"), include_str!("param_binds_planner.rb")).unwrap(); + let output = emit_and_run::ruby().arg("planner_gate.rb").current_dir(&*dir) + .env("PLANNER_BINDS", if bound { "1" } else { "0" }).output().unwrap(); + assert!(output.status.success(), "{}\n{}\n{}", dir.display(), + String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr)); + print!("{}", String::from_utf8_lossy(&output.stdout)); + std::fs::remove_dir_all(dir.parent().unwrap()).unwrap(); +} diff --git a/tests/param_binds_raw_where.rb b/tests/param_binds_raw_where.rb index 547de9168..9289d4c18 100644 --- a/tests/param_binds_raw_where.rb +++ b/tests/param_binds_raw_where.rb @@ -2,8 +2,8 @@ # never become the input to a later placeholder or replacement expansion. Db.with_connection do backslashes = %q{path\1\&\`\'} + "雪" - Db.exec("INSERT INTO items (id, parent_id, name) VALUES (1, 1, 'a'), (2, 1, 'what?')") - Db.exec("INSERT INTO items (id, parent_id, name) VALUES (3, 2, " + Db.escape_string(backslashes) + ")") + Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (1, 1, 'a', 0), (2, 1, 'what?', 0)") + Db.exec("INSERT INTO items (id, parent_id, name, required_flag) VALUES (3, 2, " + Db.escape_string(backslashes) + ", 0)") scalar = ActiveRecord::Relation.new(Item).where("items.name = ? OR items.id = ?", "what?", 1) raise "scalar value consumed later placeholder" unless scalar.count == 2 diff --git a/tests/param_binds_runtime.rb b/tests/param_binds_runtime.rb index d04f1c7f3..be6caeaeb 100644 --- a/tests/param_binds_runtime.rb +++ b/tests/param_binds_runtime.rb @@ -130,6 +130,45 @@ def bind_ephemeral(stmt) Db.finalize(stmt) puts "runtime: quotes, UTF-8, long text, copy ownership and GC passed" +# Reads must match the values Db.exec writes inline, not merely round-trip +# through the bind API. In particular SQLite TEXT and BLOB with identical +# bytes are not equal, and ASCII-only BINARY strings are written as TEXT. +Db.exec("CREATE TABLE bind_string_rows (value TEXT NOT NULL)") +def inline_bound_string(label, value) + Db.exec("DELETE FROM bind_string_rows") + Db.exec("INSERT INTO bind_string_rows VALUES (" + Db.escape_string(value) + ")") + stored = Db.prepare("SELECT typeof(value), hex(value) FROM bind_string_rows") + raise "missing inline string" if !Db.step?(stored) + storage_type = Db.column_text(stored, 0) + storage_bytes = Db.column_text(stored, 1) + Db.finalize(stored) + + inline = Db.prepare("SELECT COUNT(*) FROM bind_string_rows WHERE value = " + Db.escape_string(value)) + raise "missing inline count" if !Db.step?(inline) + expect_int(label + " inline lookup", 1, Db.column_int(inline, 0)) + Db.finalize(inline) + bound = Db.prepare("SELECT COUNT(*) FROM bind_string_rows WHERE value = ?") + Db.bind_text(bound, 1, value) + raise "missing bound count" if !Db.step?(bound) + expect_int(label + " bound lookup", 1, Db.column_int(bound, 0)) + Db.finalize(bound) + + bytes = Db.prepare("SELECT typeof(?), hex(?)") + Db.bind_text(bytes, 1, value) + Db.bind_text(bytes, 2, value) + raise "missing bound bytes" if !Db.step?(bytes) + expect_text(label + " storage class", storage_type, Db.column_text(bytes, 0)) + expect_text(label + " stored bytes", storage_bytes, Db.column_text(bytes, 1)) + Db.finalize(bytes) +end +inline_bound_string("quotes", "quote's \"double\" ? -- SQL") +inline_bound_string("UTF-8", "雪 café 🦀") +inline_bound_string("ASCII binary", "plain-ascii".b) +inline_bound_string("empty binary", "".b) +inline_bound_string("UTF-8 binary", "café".b) +inline_bound_string("invalid UTF-8 binary", "\xFF\xFE'".b) +puts "runtime: inline writes and bound reads agree on text/binary storage and bytes" + # Observe bytes as a BLOB, not SQLite length(TEXT) or column_text's C-string # conversion: SQLite string expressions on embedded NUL are not specified. # The bound value must preserve all three bytes (61 00 62). @@ -139,3 +178,20 @@ def bind_ephemeral(stmt) expect_text("embedded NUL bytes", "610062", Db.column_text(stmt, 0)) Db.finalize(stmt) puts "runtime: embedded NUL bytes passed" + +# The primitive must keep nil distinct from false even if a caller bypasses +# the lowerer's nullable inline path. Alternate on one cached shape as well. +def expect_bound_bool(value, expected) + stmt = Db.prepare("SELECT COALESCE(?, -7)") + Db.bind_bool(stmt, 1, value) + raise "missing bool row" if !Db.step?(stmt) + expect_int("nullable bool", expected, Db.column_int(stmt, 0)) + Db.finalize(stmt) +end + +expect_bound_bool(false, 0) +expect_bound_bool(nil, -7) +expect_bound_bool(true, 1) +expect_bound_bool(nil, -7) +expect_bound_bool(false, 0) +puts "runtime: nullable boolean preserves SQL NULL passed" diff --git a/tests/param_binds_text_cleanup.rb b/tests/param_binds_text_cleanup.rb new file mode 100644 index 000000000..8e0f86a49 --- /dev/null +++ b/tests/param_binds_text_cleanup.rb @@ -0,0 +1,35 @@ +# Run with either CRuby's or JRuby's Db shim preloaded via -r. +def cleanup_owned_count + conn = Db.current_dbh + if RUBY_ENGINE == "jruby" + conn.open_statements.size + else + Db.open_statements(conn).size + end +end + +class CleanupBadText + def to_s + raise "text conversion failed" + end +end + +Db.configure(":memory:") +Db.with_connection do + ["x".encode("UTF-16LE"), CleanupBadText.new].each do |value| + 3.times do + stmt = Db.prepare("SELECT ? AS preprocessing_failure") + begin + Db.bind_text(stmt, 1, value) + raise "expected text preprocessing failure" + rescue StandardError => error + raise error unless error.message.include?("incompatible character encodings") || error.message.include?("text conversion failed") + end + count = cleanup_owned_count + raise "text preprocessing left #{count} owned statements inside the lease" unless count == 0 + Db.finalize(stmt) # Idempotent after the binder's own failure cleanup. + end + end +end +puts "cleanup: 6 text preprocessing failures; every owned count was zero before lease end" +Db.close diff --git a/tests/param_binds_values.rb b/tests/param_binds_values.rb new file mode 100644 index 000000000..d729b8d64 --- /dev/null +++ b/tests/param_binds_values.rb @@ -0,0 +1,53 @@ +def expect_count(label, expected, actual) + raise "#{label}: expected #{expected}, got #{actual}" if actual != expected +end + +# Exercise the emitted inline writers, not an independently formatted seed. +# The two nearby timestamps distinguish preserved microseconds from to_s. +t1 = Time.at(1_700_000_000, 123456).getlocal("-04:00") +t2 = Time.at(1_700_000_000, 123457).getlocal("+09:00") +values = [0.0, -1.25, 1.23456789012345, 1.25e-20, 1.25e20, -1.25e20, + 1.7976931348623157e308, -1.7976931348623157e308, + 2.2250738585072014e-308, 4.9406564584124654e-324] +values.each_with_index do |value, index| + row = Reading.new + row.ratio = value + row.optional_ratio = value + row.recorded_at = t1 + row.optional_at = t2 + row.save! + expect_count("float #{index}", 1, row.ratio_matches) + expect_count("optional float #{index}", 1, row.optional_ratio_matches) + expect_count("scalar float against nullable column #{index}", 1, row.scalar_optional_ratio_matches) + expect_count("nullable float against required column #{index}", 1, row.required_ratio_matches_value(value)) + expect_count("timestamp #{index}", index + 1, row.time_matches) + expect_count("optional timestamp #{index}", index + 1, row.optional_time_matches) + expect_count("mixed predicate #{index}", 1, row.pair_matches) +end + +row = Reading.new +row.ratio = 42.5 +row.recorded_at = t2 +row.save! +expect_count("nearby timestamp", 1, row.time_matches) +expect_count("nil float", 1, row.optional_ratio_matches) +expect_count("nil timestamp", 1, row.optional_time_matches) +expect_count("nil against required float", 0, row.required_ratio_matches_value(nil)) +expect_count("nil against required timestamp", 0, row.required_time_matches_value(nil)) +expect_count("value against required timestamp", 1, row.required_time_matches_value(t2)) + +# The same instant expressed in another timezone matches the stored value; +# the adjoining microsecond remains a separate row. +row.recorded_at = t1.getutc +expect_count("same instant UTC", values.length, row.time_matches) +row.recorded_at = t2.getutc +expect_count("microsecond retained", 1, row.time_matches) +expect_count("direct offset Time", values.length, + row.time_matches_value(Time.at(1_700_000_000, 123456).getlocal("-04:00"))) +puts "typed values: 10 scalar-to-nullable floats, Float/Time nil and non-nil against nullable/required columns, UTC/offset/microseconds, mixed predicates passed" + +expect_count("String timestamp stays text", values.length, row.string_time_matches) +expect_count("optional String timestamp", values.length, + row.string_time_matches_value("2023-11-14 22:13:20.123457")) +expect_count("nil String timestamp", 1, row.string_time_matches_value(nil)) +puts "typed values: String timestamp filters stay text; optional String binds NULL" diff --git a/tests/param_binds_values.rs b/tests/param_binds_values.rs new file mode 100644 index 000000000..df9a68034 --- /dev/null +++ b/tests/param_binds_values.rs @@ -0,0 +1,310 @@ +//! Float/temporal read contract for roundhouse#12. Values serialize the same +//! way before inline escaping or binding. Run each compiler flag in its own +//! process, like param_binds.rs. + +#[path = "support/emit_and_run.rs"] +mod emit_and_run; + +use roundhouse::project::BuildTarget; +use std::process::Command; + +fn overlay() -> emit_and_run::Overlay { + emit_and_run::empty_app() + .write( + "sig/reading.rbs", + "class Reading\n def time_matches_value: (Time value) -> Integer\n def required_time_matches_value: (Time? value) -> Integer\n def required_ratio_matches_value: (Float? value) -> Integer\n def string_time_matches_value: (String? value) -> Integer\nend\n", + ) + .write( + "app/models/application_record.rb", + "class ApplicationRecord < ActiveRecord::Base\n self.abstract_class = true\nend\n", + ) + .write( + "app/controllers/application_controller.rb", + "class ApplicationController < ActionController::Base\nend\n", + ) + .write( + "config/routes.rb", + "Rails.application.routes.draw do\nend\n", + ) + .write( + "db/schema.rb", + r#" +ActiveRecord::Schema[8.1].define(version: 1) do + create_table "readings", force: :cascade do |t| + t.float "ratio", null: false + t.float "optional_ratio" + t.datetime "recorded_at", null: false + t.datetime "optional_at" + end +end +"#, + ) + .write( + "app/models/reading.rb", + r#" +class Reading < ApplicationRecord + def ratio_matches + @number = ratio.to_f + Reading.where(ratio: @number).count + end + def optional_ratio_matches + Reading.where(optional_ratio: @optional_ratio).count + end + def scalar_optional_ratio_matches + @number = ratio.to_f + Reading.where(optional_ratio: @number).count + end + def required_ratio_matches_value(value) + @nullable_number = value + Reading.where(ratio: @nullable_number).count + end + def time_matches + @needle = recorded_at + Reading.where(recorded_at: @needle).count + end + def optional_time_matches + @optional_needle = optional_at + Reading.where(optional_at: @optional_needle).count + end + def time_matches_value(value) + @bound_time = value + Reading.where(recorded_at: @bound_time).count + end + def required_time_matches_value(value) + @nullable_time = value + Reading.where(recorded_at: @nullable_time).count + end + def string_time_matches + @string_needle = "2023-11-14 22:13:20.123456" + Reading.where(recorded_at: @string_needle).count + end + def string_time_matches_value(value) + @optional_string_needle = value + Reading.where(optional_at: @optional_string_needle).count + end + def pair_matches + @key = id.to_i + @number = ratio.to_f + @needle = recorded_at + Reading.where(id: @key, ratio: @number, recorded_at: @needle).count + end +end +"#, + ) +} + +fn success(command: &mut Command) { + let output = command + .output() + .unwrap_or_else(|e| panic!("{command:?}: {e}")); + assert!( + output.status.success(), + "{command:?}: {}\n{}\n{}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + print!("{}", String::from_utf8_lossy(&output.stdout)); +} + +fn emitted(test: &str, target: BuildTarget) { + if std::env::var_os("ROUNDHOUSE_BINDS_CHILD").is_none() { + for mode in ["0", "1"] { + println!("{test}: ROUNDHOUSE_PARAM_BINDS={mode}"); + success( + Command::new(std::env::current_exe().unwrap()) + .args(["--exact", test, "--include-ignored", "--nocapture"]) + .env("ROUNDHOUSE_BINDS_CHILD", "1") + .env("ROUNDHOUSE_PARAM_BINDS", mode), + ); + } + return; + } + let (dir, errors) = overlay().emit(target); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let source = std::fs::read_to_string(dir.join("app/models/reading.rb")).unwrap(); + for method in [ + "ratio_matches", + "optional_ratio_matches", + "scalar_optional_ratio_matches", + "required_ratio_matches_value(value)", + "time_matches", + "optional_time_matches", + "time_matches_value(value)", + "required_time_matches_value(value)", + "string_time_matches", + "string_time_matches_value(value)", + "pair_matches", + ] { + let header = format!(" def {method}\n"); + let body = source + .split_once(&header) + .unwrap() + .1 + .split_once("\n end") + .unwrap() + .0; + assert!(body.contains("Db.prepare("), "{header}{body}"); + assert!(!body.contains("Reading.where("), "{header}{body}"); + if method.starts_with("optional_") && std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1" { + assert!(body.contains(" IS NULL"), "{header}{body}"); + assert!(body.contains(" = ?"), "{header}{body}"); + assert!(!body.contains(" IS ?"), "{header}{body}"); + assert!(body.contains("Db.bind_text_opt("), "{header}{body}"); + } + if method == "ratio_matches" || method == "scalar_optional_ratio_matches" { + assert!(body.contains("@number.to_s"), "{header}{body}"); + let bind = if method == "ratio_matches" { "Db.bind_text(" } else { "Db.bind_text_opt(" }; + assert_eq!( + body.contains(bind), + std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1", + "{header}{body}" + ); + } + if method == "required_ratio_matches_value(value)" { + assert!(body.contains(".to_s"), "nullable Float arguments must serialize before binding: {header}{body}"); + } + if method.starts_with("string_time") { + assert!(!body.contains("ActiveSupport.format_db_time("), "{header}{body}"); + let bind = if method == "string_time_matches" { "Db.bind_text(" } else { "Db.bind_text_opt(" }; + assert_eq!(body.contains(bind), std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1", "{header}{body}"); + } else if method.contains("time") || method == "pair_matches" { + assert!( + body.contains("ActiveSupport.format_db_time("), + "{header}{body}" + ); + } + if method == "time_matches_value(value)" { + assert_eq!( + body.contains("Db.bind_text("), + std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1", + "{header}{body}" + ); + } + if method == "pair_matches" && std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1" { + assert!( + body.contains("Db.bind_int(stmt, 1, @key)"), + "{header}{body}" + ); + assert!( + body.contains("Db.bind_text(stmt, 2, @number.to_s)"), + "{header}{body}" + ); + assert!( + body.contains("Db.bind_text(stmt, 3, ActiveSupport.format_db_time(@needle))"), + "{header}{body}" + ); + } + } + let script = format!( + r#"require_relative "boot" +require_relative "app/models/reading" +SqliteAdapter.configure("file:bind_values?mode=memory&cache=shared") +ActiveRecord.adapter = SqliteAdapter +Schema.statements.each {{ |sql| Db.exec(sql) }} +{} +Db.close +"#, + include_str!("param_binds_values.rb") + ); + std::fs::write(dir.join("bind_values.rb"), script).unwrap(); + println!("value gate tree: {}", dir.display()); + if target == BuildTarget::Spinel { + success( + Command::new(std::env::var("SPINEL").unwrap_or_else(|_| "spinel".into())) + .args(["bind_values.rb", "-o", "bind_values"]) + .current_dir(&*dir), + ); + success(Command::new(dir.join("bind_values")).current_dir(&*dir)); + } else { + success(Command::new("ruby").arg("bind_values.rb").current_dir(&*dir)); + } +} + +#[test] +fn typed_values_ruby() { + emitted("typed_values_ruby", BuildTarget::Ruby); +} + +#[test] +#[ignore = "requires Spinel (SPINEL=/path/to/spinel)"] +fn typed_values_spinel() { + emitted("typed_values_spinel", BuildTarget::Spinel); +} + +#[test] +fn string_temporal_ivar_stays_a_string_on_strict_targets() { + for (target, path, method, forbidden) in [ + (BuildTarget::Go, "app/v2/article.go", "SinceStringCount", "Rh_format_db_time(self.Since)"), + (BuildTarget::Rust, "src/models/article.rs", "since_string_count", "self.since.map(crate::rh_datetime::format_db_time)"), + ] { + let (dir, errors) = emit_and_run::real_blog().edit( + "app/models/article.rb", + "class Article < ApplicationRecord", + "class Article < ApplicationRecord\n def since_string_count\n @since = \"2020-01-01 00:00:00.000000\"\n Article.where(created_at: @since).count\n end", + ).emit(target); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let source = std::fs::read_to_string(dir.join(path)).unwrap(); + assert!(source.contains(method), "{target:?}: missing method"); + assert!(!source.contains(forbidden), "{target:?}: a String was passed to the native time formatter"); + } +} + +#[test] +fn date_predicates_ruby() { + if std::env::var_os("ROUNDHOUSE_BINDS_CHILD").is_none() { + for mode in ["0", "1"] { + success(Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "date_predicates_ruby", "--nocapture"]) + .env("ROUNDHOUSE_BINDS_CHILD", "1").env("ROUNDHOUSE_PARAM_BINDS", mode)); + } + return; + } + let (dir, errors) = emit_and_run::empty_app() + .write("app/models/application_record.rb", "class ApplicationRecord < ActiveRecord::Base\n self.abstract_class = true\nend\n") + .write("app/controllers/application_controller.rb", "class ApplicationController < ActionController::Base\nend\n") + .write("config/routes.rb", "Rails.application.routes.draw do\nend\n") + .write("db/schema.rb", "ActiveRecord::Schema[8.1].define(version: 1) do\n create_table :calendar_entries do |t|\n t.date :due_on\n end\nend\n") + .write("sig/calendar_entry.rbs", "class CalendarEntry\n def date_matches: (Date? value) -> Integer\n def text_matches: (String? value) -> Integer\nend\n") + .write("app/models/calendar_entry.rb", r#" +class CalendarEntry < ApplicationRecord + def date_matches(value) + @date_needle = value + CalendarEntry.where(due_on: @date_needle).count + end + def text_matches(value) + @text_needle = value + CalendarEntry.where(due_on: @text_needle).count + end +end +"#).emit(BuildTarget::Ruby); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let source = std::fs::read_to_string(dir.join("app/models/calendar_entry.rb")).unwrap(); + assert!(source.contains("ActiveSupport.format_db_date(@date_needle)"), "{source}"); + assert!(!source.contains("ActiveSupport.format_db_date(@text_needle)"), "{source}"); + if std::env::var("ROUNDHOUSE_PARAM_BINDS").unwrap() == "1" { + assert!(source.contains("Db.bind_text_opt("), "{source}"); + assert!(source.contains(" IS NULL"), "{source}"); + assert!(source.contains(" = ?"), "{source}"); + } + std::fs::write(dir.join("date_gate.rb"), r#" +require_relative "boot" +require_relative "app/models/calendar_entry" +SqliteAdapter.configure(":memory:") +ActiveRecord.adapter = SqliteAdapter +Schema.statements.each { |sql| Db.exec(sql) } +date = Date.new(2024, 2, 29) +row = CalendarEntry.new +row.due_on = date +row.save! +CalendarEntry.new.save! +raise "Date value" unless row.date_matches(date) == 1 +raise "Date NULL" unless row.date_matches(nil) == 1 +raise "String date" unless row.text_matches("2024-02-29") == 1 +raise "String NULL" unless row.text_matches(nil) == 1 +raise "different date" unless row.date_matches(Date.new(2024, 3, 1)) == 0 +puts "typed values: native Ruby Date/String and NULL predicates passed" +Db.close +"#).unwrap(); + success(Command::new("ruby").arg("date_gate.rb").current_dir(&*dir)); +}