-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[prepared statements] fix for auto_encode_arrays, refactoring #59
Changes from 5 commits
6dc5312
b89a2e4
7fc32ce
36635d7
35e3eb6
a792b16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,19 +51,21 @@ def quoted_time(value) | |
value.utc.iso8601 | ||
end | ||
|
||
EMPTY_ARRAY = [].freeze | ||
|
||
def quote_val(value) | ||
case value | ||
when String then "'#{conn.escape_string(value.to_s)}'" | ||
when Numeric then value.to_s | ||
when BigDecimal then value.to_s("F") | ||
when Time then "'#{quoted_time(value)}'" | ||
when Date then "'#{value.to_s}'" | ||
when Symbol then "'#{conn.escape_string(value.to_s)}'" | ||
when true then "true" | ||
when false then "false" | ||
when nil then "NULL" | ||
when [] then "NULL" | ||
when Array then array_encoder ? "'#{array_encoder.encode(value)}'" : value.map { |v| quote_val(v) }.join(', ') | ||
when String then "'#{conn.escape_string(value.to_s)}'" | ||
when Numeric then value.to_s | ||
when BigDecimal then value.to_s("F") | ||
when Time then "'#{quoted_time(value)}'" | ||
when Date then "'#{value.to_s}'" | ||
when Symbol then "'#{conn.escape_string(value.to_s)}'" | ||
when true then "true" | ||
when false then "false" | ||
when nil then "NULL" | ||
when EMPTY_ARRAY then array_encoder ? "'{}'" : "NULL" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. arr = []
freeze_arr = [].freeze
Benchmark.ips do |x|
x.report("new") {
case arr
when [] then true
else false
end
}
x.report("inline freeze") {
case arr
when [].freeze then true
else false
end
}
x.report("var freeze") {
case arr
when freeze_arr then true
else false
end
}
x.compare!
end;
# Comparison:
# var freeze: 6412829.6 i/s
# new: 5822569.7 i/s - 1.10x slower
# inline freeze: 5379263.0 i/s - 1.19x slower |
||
when Array then array_encoder ? "'#{array_encoder.encode(value)}'" : value.map { |v| quote_val(v) }.join(', ') | ||
else raise TypeError, "can't quote #{value.class.name}" | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# frozen_string_literal: true | ||
|
||
require "mini_sql/abstract/prepared_binds" | ||
|
||
module MiniSql | ||
module Postgres | ||
class PreparedBindsAutoArray < ::MiniSql::Abstract::PreparedBinds | ||
|
||
attr_reader :array_encoder | ||
|
||
def initialize(array_encoder) | ||
@array_encoder = array_encoder | ||
end | ||
|
||
def bind_hash(sql, hash) | ||
sql = sql.dup | ||
binds = [] | ||
bind_names = [] | ||
i = 0 | ||
|
||
hash.each do |k, v| | ||
binds << (v.is_a?(Array) ? array_encoder.encode(v) : v) | ||
bind_names << [BindName.new(k)] | ||
bind_outputs = bind_output(i += 1) | ||
|
||
sql.gsub!(":#{k}") do | ||
# ignore ::int and stuff like that | ||
# $` is previous to match | ||
if $` && $`[-1] != ":" | ||
bind_outputs | ||
else | ||
":#{k}" | ||
end | ||
end | ||
end | ||
[sql, binds, bind_names] | ||
end | ||
|
||
def bind_array(sql, array) | ||
sql = sql.dup | ||
param_i = -1 | ||
i = 0 | ||
binds = [] | ||
bind_names = [] | ||
sql.gsub!("?") do | ||
v = array[param_i += 1] | ||
binds << (v.is_a?(Array) ? array_encoder.encode(v) : v) | ||
i += 1 | ||
bind_names << [BindName.new("$#{i}")] | ||
bind_output(i) | ||
end | ||
[sql, binds, bind_names] | ||
end | ||
|
||
def bind_output(i) | ||
"$#{i}" | ||
end | ||
|
||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,13 +7,9 @@ class PreparedConnection < Connection | |
attr_reader :unprepared | ||
|
||
def initialize(unprepared_connection) | ||
@unprepared = unprepared_connection | ||
@raw_connection = unprepared_connection.raw_connection | ||
@type_map = unprepared_connection.type_map | ||
@param_encoder = unprepared_connection.param_encoder | ||
|
||
@prepared_cache = PreparedCache.new(@raw_connection) | ||
@param_binder = PreparedBinds.new | ||
@unprepared = unprepared_connection | ||
@type_map = unprepared_connection.type_map | ||
@param_binder = unprepared.array_encoder ? PreparedBindsAutoArray.new(unprepared.array_encoder) : PreparedBinds.new | ||
end | ||
|
||
def build(_) | ||
|
@@ -30,8 +26,9 @@ def deserializer_cache | |
|
||
private def run(sql, params) | ||
prepared_sql, binds, _bind_names = @param_binder.bind(sql, *params) | ||
@prepared_cache ||= PreparedCache.new(unprepared) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lazy evaluate |
||
prepare_statement_key = @prepared_cache.prepare_statement(prepared_sql) | ||
raw_connection.exec_prepared(prepare_statement_key, binds) | ||
unprepared.raw_connection.exec_prepared(prepare_statement_key, binds) | ||
end | ||
|
||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# frozen_string_literal: true | ||
|
||
require "test_helper" | ||
require_relative 'connection_test' | ||
require_relative 'prepared_connection_test' | ||
|
||
module MiniSql::Postgres::ArrayTests | ||
def test_simple_params | ||
nums, strings, empty_array = [1, 2, 3], %w[a b c], [] | ||
|
||
row = @connection.query_single("select ?::int[], ?::text[], ?::int[]", nums, strings, empty_array) | ||
|
||
assert_equal(row, [nums, strings, empty_array]) | ||
end | ||
|
||
def test_hash_params | ||
nums, strings, empty_array = [1, 2, 3], %w[a b c], [] | ||
|
||
row = @connection.query_single("select :nums::int[], :strings::text[], :empty_array::int[]", nums: nums, strings: strings, empty_array: empty_array) | ||
|
||
assert_equal(row, [nums, strings, empty_array]) | ||
end | ||
end | ||
|
||
class MiniSql::Postgres::TestAutoEncodeArraysPrepared < MiniSql::Postgres::TestPreparedConnection | ||
include MiniSql::Postgres::ArrayTests | ||
|
||
def setup | ||
@unprepared_connection = pg_connection(auto_encode_arrays: true) | ||
@connection = @unprepared_connection.prepared | ||
|
||
setup_tables | ||
end | ||
end | ||
|
||
class MiniSql::Postgres::TestAutoEncodeArraysUnprepared < MiniSql::Postgres::TestConnection | ||
include MiniSql::Postgres::ArrayTests | ||
|
||
def setup | ||
@connection = pg_connection(auto_encode_arrays: true) | ||
end | ||
end | ||
Comment on lines
+25
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. run all test for connection with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for usage with
ActiveRecord