Skip to content

Fix: bind parameters by position to fix BETWEEN and other duplicate-name binds - #20

Merged
kou merged 1 commit into
red-data-tools:mainfrom
suaron:fix/between-bind-overwrite
Jun 16, 2026
Merged

Fix: bind parameters by position to fix BETWEEN and other duplicate-name binds#20
kou merged 1 commit into
red-data-tools:mainfrom
suaron:fix/between-bind-overwrite

Conversation

@suaron

@suaron suaron commented May 8, 2026

Copy link
Copy Markdown
Contributor

Summary

When a query reuses one column across more than one ? placeholder, the adapter sent only one value to the driver, so the driver rejected the prepared statement (it expected N values and got 1).

This affects any where with a range (Arel::Between), an IN array, or a raw SQL fragment that names the same column twice.

The fix stops duplicate column names from overwriting each other.

Reproduction

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "activerecord", "~> 8.1"
  gem "activerecord-adbc-adapter", "0.0.1"
end

require "active_record"
require "activerecord-adbc-adapter"

ActiveRecord::Base.establish_connection(
  adapter: "adbc",
  driver:  "adbc_driver_sqlite",
  uri:     "file::memory:?cache=shared",
)

conn = ActiveRecord::Base.connection
conn.execute(<<~SQL)
  CREATE TABLE sales (
    id      INTEGER PRIMARY KEY,
    sold_on DATE,
    region  TEXT,
    amount  REAL
  )
SQL

conn.execute(<<~SQL)
  INSERT INTO sales (id, sold_on, region, amount) VALUES
    (1, '2025-01-15', 'US',   10.0),
    (2, '2025-02-15', 'EU',   20.0),
    (3, '2025-03-15', 'APAC', 30.0),
    (4, '2025-05-15', 'US',   40.0)
SQL

class Sale < ActiveRecord::Base
  self.table_name = "sales"
end

RULE = "─" * 60

def shorten_frame(frame)
  frame
    .sub(%r{.*/gems/}, "")
    .sub(%r{.*/lib/ruby/[^/]+/}, "")
end

def check(label, &block)
  result = block.call
  puts "✓ #{label}#{result.inspect}"
  puts RULE
rescue => e
  puts "✗ #{label}"
  e.message.lines.each {|line| puts "↳ #{line.chomp}" }
  e.backtrace.first(3).each {|frame| puts " at #{shorten_frame(frame)}" }
  puts RULE
end

puts "running bind-binding checks against ADBC SQLite"

check("BETWEEN inclusive range") do
  Sale.where(sold_on: Date.new(2025, 1, 1)..Date.new(2025, 3, 31)).count
end

check("BETWEEN exclusive range") do
  Sale.where(sold_on: Date.new(2025, 1, 1)...Date.new(2025, 3, 31)).count
end

check("IN array") do
  Sale.where(id: [1, 2, 3]).count
end

check("raw SQL repeated placeholder") do
  Sale.where("region = ? OR region = ?", "US", "EU").count
end

check("range + raw fragment") do
  Sale.where(sold_on: Date.new(2025, 1, 1)..Date.new(2025, 12, 31))
      .where("amount < ?", 25.0)
      .count
end

Root cause

ActiveRecordADBCAdapter::DatabaseStatements#perform_query keyed the parameter RecordBatch by bind.name:

binds.zip(type_casted_binds) do |bind, type_casted_bind|
  # ... build Arrow array ...
  raw_records[bind.name] = array      # collides on duplicate name
end
record_batch = Arrow::RecordBatch.new(raw_records)
statement.bind(record_batch) { statement.execute[0] }

where(sold_on: from..to) emits two QueryAttribute binds, both named "sold_on", for:

WHERE "sales"."sold_on" BETWEEN ? AND ?

On the second iteration the hash assignment overwrites the first. The RecordBatch ends up with one column while the prepared statement still needs two values.

The fix

Key columns by position and append the bind name only for readability:

binds.zip(type_casted_binds).each_with_index do |(bind, type_casted_bind), i|
  # ... build Arrow array ...
  # Some binds (e.g. BETWEEN range values) aren't QueryAttributes and
  # have no #name. Position `i` is the real key; name just aids debugging.
  name = bind.respond_to?(:name) ? bind.name : "p"
  raw_records["#{i}_#{name}"] = array
end

The position prefix keeps keys unique (0_sold_on, 1_sold_on, ...) even when two binds share a name. The name suffix is only there to make the columns readable when debugging. Ruby preserves hash insertion order, so column order still matches the order of the ? placeholders.

Switching to binds.zip(type_casted_binds) also fixes a separate crash: raw SQL fragments like where("col >= ?", 5) arrive in binds as plain values, not QueryAttribute objects, so .name raised NoMethodError. The respond_to?(:name) guard falls back to "p", and the value comes from type_casted_binds, so the loop no longer cares about the bind's shape.

Test plan

New cases in test/test_model.rb:

  • test_where_range_betweenwhere(id: 1..2).count
  • test_where_range_between_exclusive_endwhere(id: 1...2).count
  • test_where_raw_sql_fragmentwhere("id >= ?", 2)
  • test_where_in_arraywhere(id: [1, 2, 3])
  • test_where_mixed_range_and_rawwhere(id: 1..2).where("id < ?", 2)
  • test_limitorder(:id).limit(2) returns 2 rows
  • test_offsetorder(:id).limit(2).offset(2) returns the trailing row

CI runs the suite three times via ACTIVERECORD_ADBC_ADAPTER_BACKEND (sqlite default, postgresql, duckdb). All cases pass on each backend.

Comment thread lib/activerecord_adbc_adapter/database_statements.rb Outdated
Comment thread lib/activerecord_adbc_adapter/database_statements.rb

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Comment thread lib/activerecord_adbc_adapter/database_statements.rb
@suaron
suaron force-pushed the fix/between-bind-overwrite branch from e420f30 to a9d76b8 Compare June 10, 2026 12:32
@suaron
suaron requested a review from kou June 10, 2026 12:40
@kou

kou commented Jun 14, 2026

Copy link
Copy Markdown
Member

Could you update the PR description to reflect the latest change? We'll use the PR description as a commit message.

Reusing one column across multiple `?` placeholders sent only one value to
the driver, so the prepared statement failed. Affects `where` with a range,
IN array, or raw SQL naming a column twice.

Cause: RecordBatch was keyed by `bind.name`, so duplicate names overwrote
each other. Now keyed by position (`"#{i}_#{name}"`); name is debug-only.
Also guards `.name` with `respond_to?` — raw SQL binds are plain values,
not QueryAttributes.

Tests: between, IN, raw fragment, range+raw, limit, offset.
@suaron
suaron force-pushed the fix/between-bind-overwrite branch from a9d76b8 to 1c8506a Compare June 16, 2026 10:26
@suaron

suaron commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Could you update the PR description to reflect the latest change? We'll use the PR description as a commit message.

PR description updated to reflect the latest change and tidied up for use as the commit message.

@kou
kou merged commit 11ac630 into red-data-tools:main Jun 16, 2026
4 checks passed
@kou

kou commented Jun 16, 2026

Copy link
Copy Markdown
Member

Thanks.

@suaron
suaron deleted the fix/between-bind-overwrite branch June 16, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants