From 3598dddb655f01035ab09116005a072a6a6b91ed Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Mon, 1 Feb 2016 23:02:04 -0300 Subject: [PATCH] split perform_query and perform_exec add DB::ExecResult to have a concurrency safety exec result remove connection string from abstract DB::Connection --- spec/std/db/db_spec.cr | 14 -------------- spec/std/db/dummy_driver.cr | 24 ++++++++++++++++-------- src/db/connection.cr | 8 -------- src/db/db.cr | 4 +++- src/db/query_methods.cr | 17 ++++++++++------- src/db/result_set.cr | 5 ----- src/db/statement.cr | 32 ++++++++++++++++++-------------- 7 files changed, 47 insertions(+), 57 deletions(-) diff --git a/spec/std/db/db_spec.cr b/spec/std/db/db_spec.cr index d8a3e8b..63171bc 100644 --- a/spec/std/db/db_spec.cr +++ b/spec/std/db/db_spec.cr @@ -36,24 +36,10 @@ describe DB do end end - it "exec should close statement" do - with_dummy do |db| - db.exec "" - DummyDriver::DummyResultSet.last_result_set.closed?.should be_true - end - end - it "scalar should close statement" do with_dummy do |db| db.scalar "1" DummyDriver::DummyResultSet.last_result_set.closed?.should be_true end end - - it "exec should perform statement" do - with_dummy do |db| - db.exec "" - DummyDriver::DummyResultSet.last_result_set.executed?.should be_true - end - end end diff --git a/spec/std/db/dummy_driver.cr b/spec/std/db/dummy_driver.cr index a1a4433..2845c92 100644 --- a/spec/std/db/dummy_driver.cr +++ b/spec/std/db/dummy_driver.cr @@ -6,6 +6,11 @@ class DummyDriver < DB::Driver end class DummyConnection < DB::Connection + getter connection_string + + def initialize(@connection_string) + end + def prepare(query) DummyStatement.new(self, query) end @@ -26,12 +31,21 @@ class DummyDriver < DB::Driver super(driver) end - protected def perform(args : Slice(DB::Any)) + protected def perform_query(args : Slice(DB::Any)) + set_params args + DummyResultSet.new self, @query + end + + protected def perform_exec(args : Slice(DB::Any)) + set_params args + DB::ExecResult.new 0, 0 + end + + private def set_params(args) @params.clear args.each_with_index do |arg, index| @params[index] = arg end - DummyResultSet.new self, @query end end @@ -42,7 +56,6 @@ class DummyDriver < DB::Driver super(statement) @iterator = query.split.map { |r| r.split(',') }.to_a.each - @executed = false @@last_result_set = self end @@ -50,12 +63,7 @@ class DummyDriver < DB::Driver @@last_result_set.not_nil! end - def executed? - @executed - end - def move_next - @executed = true @iterator.next.tap do |n| return false if n.is_a?(Iterator::Stop) @values = n.each diff --git a/src/db/connection.cr b/src/db/connection.cr index 9ac0900..f409b1c 100644 --- a/src/db/connection.cr +++ b/src/db/connection.cr @@ -17,12 +17,8 @@ module DB abstract class Connection # TODO add IDLE status, for connection ppool management. - getter connection_string # TODO Remove @closed = false - def initialize(@connection_string) # TODO Remove - end - # Closes this connection. def close raise "Connection already closed" if @closed # TODO make it no fail if closed @@ -45,10 +41,6 @@ module DB include QueryMethods - # Returns the last inserted id through this connection. - abstract def last_insert_id : Int64 # TODO move to ExecResult record. plano. with last_rows. eagerly askit. - - protected abstract def perform_close # TODO do_close end end diff --git a/src/db/db.cr b/src/db/db.cr index 0ad093e..0dd4842 100644 --- a/src/db/db.cr +++ b/src/db/db.cr @@ -67,8 +67,10 @@ module DB # See `DB::TYPES` in `DB`. `Any` is a nillable version of the union of all types in `DB::TYPES` alias Any = Nil | String | Int32 | Int64 | Float32 | Float64 | Slice(UInt8) - # :nodoc: + # Result of a `#exec` statement. + record ExecResult, rows_affected, last_insert_id + # :nodoc: def self.driver_class(driver_name) # : Driver.class @@drivers.not_nil![driver_name] end diff --git a/src/db/query_methods.cr b/src/db/query_methods.cr index bd92a8c..dd24ae1 100644 --- a/src/db/query_methods.cr +++ b/src/db/query_methods.cr @@ -3,19 +3,22 @@ module DB # All methods accepts a `query : String` and a set arguments. # # Three kind of statements can be performed: - # 1. `#exec` waits no response from the database. - # 2. `#scalar` reads a single value of the response. Use `#scalar?` if the response is nillable. - # 3. `#query` returns a ResultSet that allows iteration over the rows in the response and column information. + # 1. `#exec` waits no record response from the database. An `ExecResult` is returned. + # 2. `#scalar` reads a single value of the response. A `DB::Any` is returned. + # 3. `#query` returns a `ResultSet` that allows iteration over the rows in the response and column information. # - # Arguments can be passed: - # * by position: `db.query("SELECT name FROM ... WHERE age > ?", age)` - # * by symbol: `db.query("SELECT name FROM ... WHERE age > :age", {age: age})` - # * by string: `db.query("SELECT name FROM ... WHERE age > :age", {"age": age})` + # Arguments can be passed by position + # + # ``` + # db.query("SELECT name FROM ... WHERE age > ?", age) + # ``` # # Convention of mapping how arguments are mapped to the query depends on each driver. # # Including `QueryMethods` requires a `prepare(query) : Statement` method. module QueryMethods + abstract def prepare(query) : Statement + # Returns a `ResultSet` for the `query`. # The `ResultSet` must be closed manually. def query(query, *args) diff --git a/src/db/result_set.cr b/src/db/result_set.cr index d6a680c..3015d15 100644 --- a/src/db/result_set.cr +++ b/src/db/result_set.cr @@ -47,11 +47,6 @@ module DB protected def do_close end - # Ensures it executes the query - def exec - move_next - end - # Move the next row in the result. # Return `false` if no more rows are available. # See `#each` diff --git a/src/db/statement.cr b/src/db/statement.cr index 9fbbe95..0b670e1 100644 --- a/src/db/statement.cr +++ b/src/db/statement.cr @@ -6,10 +6,9 @@ module DB # # 1. Subclass `Statements` # 2. `Statements` are created from a custom driver `Connection#prepare` method. - # 3. `#begin_parameters` is called before the parameters are set. - # 4. `#add_parameter` methods helps to support 0-based positional arguments and named arguments - # 5. After parameters are set `#perform` is called to return a `ResultSet` - # 6. `#on_close` is called to release the statement resources. + # 3. `#perform_query` executes a query that is expected to return a `ResultSet` + # 4. `#perform_exec` executes a query that is expected to return an `ExecResult` + # 6. `#do_close` is called to release the statement resources. abstract class Statement getter connection @@ -17,11 +16,15 @@ module DB @closed = false end + # See `QueryMethods#exec` + def exec + perform_exec(Slice(Any).new(0)) # no overload matches ... with types Slice(NoReturn) + end + # See `QueryMethods#exec` def exec(*args) - query(*args) do |rs| - rs.exec - end + # TODO better way to do it + perform_exec(args.to_a.to_unsafe.to_slice(args.size)) end # See `QueryMethods#scalar` @@ -55,12 +58,12 @@ module DB # See `QueryMethods#query` def query(*args) - execute *args + perform_query *args end # See `QueryMethods#query` def query(*args) - execute(*args).tap do |rs| + perform_query(*args).tap do |rs| begin yield rs ensure @@ -69,13 +72,13 @@ module DB end end - private def execute : ResultSet - perform(Slice(Any).new(0)) + private def perform_query : ResultSet + perform_query(Slice(Any).new(0)) # no overload matches ... with types Slice(NoReturn) end - private def execute(*args) : ResultSet + private def perform_query(*args) : ResultSet # TODO better way to do it - perform(args.to_a.to_unsafe.to_slice(args.size)) + perform_query(args.to_a.to_unsafe.to_slice(args.size)) end # Closes this statement. @@ -95,7 +98,8 @@ module DB close end - protected abstract def perform(args : Slice(Any)) : ResultSet + protected abstract def perform_query(args : Slice(Any)) : ResultSet + protected abstract def perform_exec(args : Slice(Any)) : ExecResult protected def do_close end