diff --git a/runtime/ruby/active_record/relation.rb b/runtime/ruby/active_record/relation.rb index e5420e625..eaa5296a4 100644 --- a/runtime/ruby/active_record/relation.rb +++ b/runtime/ruby/active_record/relation.rb @@ -1817,15 +1817,25 @@ def column_predicate(col, val) end end - # Replace `?` placeholders in a raw fragment with escaped args, in - # order. A fragment with no `?` returns unchanged. Each `sub` rewrites - # the leftmost remaining `?`, so iterating the args consumes them in - # order. + # A single Hash dispatches to the named-bind scan of the original SQL. + # For positional binds, split only the original `?` placeholders, + # keeping escaped values verbatim. Preserve trailing empty parts so + # missing binds leave their `?` intact. def substitute_binds(sql, args) first = args[0] return substitute_named_binds(sql, first) if args.length == 1 && first.is_a?(Hash) - result = sql - args.each { |a| result = result.sub("?", ActiveRecord.adapter.escape_value(a)) } + parts = sql.split("?", -1) + result = parts[0].to_s + index = 0 + while index < parts.length - 1 + if index < args.length + result = result + ActiveRecord.adapter.escape_value(args[index]) + else + result = result + "?" + end + result = result + parts[index + 1].to_s + index += 1 + end result end diff --git a/scripts/ci-plan.py b/scripts/ci-plan.py index fdfef40d9..a7b2efcd1 100755 --- a/scripts/ci-plan.py +++ b/scripts/ci-plan.py @@ -108,6 +108,7 @@ def native_coverage(path): suites.add("db_sqlite_concurrency") if path in { "tests/param_binds_emit.rb", + "tests/param_binds_raw_where.rb", "tests/param_binds_runtime.rb", "tests/support/emit_and_run.rs", "src/lower/model_to_library/adapter_emit.rs", diff --git a/tests/ci_plan_test.py b/tests/ci_plan_test.py index 8c6117d1a..2e4173c8b 100644 --- a/tests/ci_plan_test.py +++ b/tests/ci_plan_test.py @@ -397,6 +397,7 @@ def test_param_binds_owns_lowering_drivers_and_database_runtime(self): "src/lower/model_to_library/adapter_emit.rs", "tests/param_binds.rs", "tests/param_binds_emit.rb", + "tests/param_binds_raw_where.rb", "tests/param_binds_runtime.rb", "tests/support/emit_and_run.rs", ]: diff --git a/tests/inference_on_spinel_blog_runtime_with_rbs.rs b/tests/inference_on_spinel_blog_runtime_with_rbs.rs index d5243228c..bcddba55d 100644 --- a/tests/inference_on_spinel_blog_runtime_with_rbs.rs +++ b/tests/inference_on_spinel_blog_runtime_with_rbs.rs @@ -601,6 +601,12 @@ fn untyped_subexpressions_with_rbs_baseline() { // Soft ratchet under canonical class-ID lookup (Fixes #435), then // dependency RBS + cross-stem ivar merge + Relation/Base overlays. // Fails only when the residual rises. Not a substitute for Bar B. + // 2026-10-06: positional raw WHERE/HAVING substitution, measured + // against main c49721be with this current RBS-seeded probe. Paired + // totals stay 0 -> 0 (relation.rb 0 -> 0), so the measured delta is + // 0 - 0 = 0 and main's committed ceiling stays 0 + 0 = 0. The older + // probe on 0ac82f51 measured +4; its ceiling does not carry forward. + // Keep the named-bind dispatch/scan and RBS signatures unchanged. const CEILING: usize = 0; assert!( diff --git a/tests/param_binds.rs b/tests/param_binds.rs index 13a2d9e55..10731d74c 100644 --- a/tests/param_binds.rs +++ b/tests/param_binds.rs @@ -372,3 +372,32 @@ fn varying_binds_spinel() { fn bind_runtime_spinel() { runtime(true); } + +fn raw_where_substitution(target: BuildTarget) { + let (dir, errors) = overlay().emit(target); + assert!(errors.is_empty(), "{}", errors.join("\n")); + let script = format!( + r#"require_relative "boot" +require_relative "app/models/item" +SqliteAdapter.configure("file:raw_where_gate?mode=memory&cache=shared") +ActiveRecord.adapter = SqliteAdapter +Schema.statements.each {{ |sql| Db.exec(sql) }} +{} +Db.close +"#, + include_str!("param_binds_raw_where.rb") + ); + run_script(&dir, &script, target == BuildTarget::Spinel); + std::fs::remove_dir_all(dir.parent().unwrap()).expect("remove successful overlay"); +} + +#[test] +fn raw_where_substitution_ruby() { + raw_where_substitution(BuildTarget::Ruby); +} + +#[test] +#[ignore = "requires Spinel (SPINEL=/path/to/spinel)"] +fn raw_where_substitution_spinel() { + raw_where_substitution(BuildTarget::Spinel); +} diff --git a/tests/param_binds_raw_where.rb b/tests/param_binds_raw_where.rb new file mode 100644 index 000000000..547de9168 --- /dev/null +++ b/tests/param_binds_raw_where.rb @@ -0,0 +1,43 @@ +# Execute raw Relation predicates against real rows. Escaped values must +# 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) + ")") + + scalar = ActiveRecord::Relation.new(Item).where("items.name = ? OR items.id = ?", "what?", 1) + raise "scalar value consumed later placeholder" unless scalar.count == 2 + escaped = ActiveRecord::Relation.new(Item).where("items.name = ? AND items.id = ?", backslashes, 3) + raise "replacement escapes changed scalar value" unless escaped.count == 1 + quoted = ActiveRecord::Relation.new(Item).where("items.name = ? OR items.id = ?", "x' OR ?=1 --", 999) + raise "quoted scalar changed predicate" unless quoted.count == 0 + reversed = ActiveRecord::Relation.new(Item).where("items.id = ? AND items.name = ?", 2, "what?") + raise "last argument changed" unless reversed.count == 1 + negated = ActiveRecord::Relation.new(Item).not("items.name = ? OR items.id = ?", "what?", 1) + raise "negated substitution changed" unless negated.count == 1 + having = ActiveRecord::Relation.new(Item).group(:id).having("items.name = ? AND items.id = ?", "what?", 2) + raise "HAVING substitution changed" unless having.load_records.length == 1 + + # Keep main's adapter treatment of Arrays, not the draft's IN-list expansion. + list = ActiveRecord::Relation.new(Item).where("items.id IN (?) AND items.parent_id = ?", [1, 2], 1) + raise "Array escaping changed" unless list.to_sql.include?("items.id IN ('[1, 2]') AND items.parent_id = 1") + empty = ActiveRecord::Relation.new(Item).where("items.id IN (?)", []) + raise "empty Array escaping changed" unless empty.to_sql.include?("items.id IN ('[]')") + hashed = ActiveRecord::Relation.new(Item).where(id: [1, 3]) + raise "hash IN changed" unless hashed.count == 2 + + # This repair does not add a SQL parser or an arity policy. + literal = ActiveRecord::Relation.new(Item).where("items.id = 1", "ignored?") + raise "literal raw clause changed" unless literal.count == 1 + surplus = ActiveRecord::Relation.new(Item).where("items.name = ?", "what?", "ignored") + raise "surplus argument rewrote substituted data" unless surplus.count == 1 + missing = ActiveRecord::Relation.new(Item).where("items.name = ? AND items.id = ?", "what?") + raise "missing argument changed placeholder" unless missing.to_sql.include?("items.name = 'what?' AND items.id = ?") + none = ActiveRecord::Relation.new(Item).where("items.id = ?") + raise "unfilled trailing placeholder changed" unless none.to_sql.include?("items.id = ?") + adjacent = ActiveRecord::Relation.new(Item).where("??", "a", "b") + raise "adjacent placeholders changed" unless adjacent.to_sql.include?("('a''b')") + blank = ActiveRecord::Relation.new(Item).where("", "ignored") + raise "empty fragment changed" if blank.to_sql.include?("WHERE") +end +puts "raw where: scalar substitution, escaping, negation, HAVING and Array compatibility passed"