Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Allow Models to use the RETURNING clause when inserting records on Po…

…stgreSQL

This commit makes model.save! check for an insert_select method on
the dataset.  If it exists, it calls that method and uses the result
as the values, instead of inserting and then reloading the values
from the database. This should be faster as well as less error prone.

It changes a recent commit to the PostgreSQL adapter so that
insert_sql does not use the RETURNING clause by default. You need to
call the insert_returning_sql method if you want the RETURNING
clause.  That is what Dataset#insert now does on PostgreSQL servers
8.2 and higher.  This caused some issues with prepared statements,
but this commit takes care of that as well.

This commit also contains a small speedup to Dataset#single_value,
as well as a missing spec for Database#raise_error.
  • Loading branch information...
commit 7350715293092a04645376b2d9e82a382e5c5cb0 1 parent 6acb504
@jeremyevans authored
View
2  CHANGELOG
@@ -1,5 +1,7 @@
=== HEAD
+* Allow Models to use the RETURNING clause when inserting records on PostgreSQL (jeremyevans)
+
* Raise Sequel::DatabaseError instead of generic Sequel::Error for database errors, don't swallow tracebacks (jeremyevans)
* Use INSERT ... RETURNING ... with PostgreSQL 8.2 and higher (jeremyevans)
View
7 lib/sequel_core/adapters/jdbc/postgresql.rb
@@ -82,6 +82,13 @@ def last_insert_id(conn, opts)
# Dataset subclass used for datasets that connect to PostgreSQL via JDBC.
class Dataset < JDBC::Dataset
include Sequel::Postgres::DatasetMethods
+
+ # Add the shared PostgreSQL prepared statement methods
+ def prepare(*args)
+ ps = super
+ ps.extend(::Sequel::Postgres::DatasetMethods::PreparedStatementMethods)
+ ps
+ end
# Convert Java::JavaSql::Timestamps correctly, and handle SQL::Blobs
# correctly.
View
1  lib/sequel_core/adapters/postgres.rb
@@ -371,6 +371,7 @@ def execute_insert(sql, opts={}, &block)
# pg driver.
module PreparedStatementMethods
include BindArgumentMethods
+ include ::Sequel::Postgres::DatasetMethods::PreparedStatementMethods
private
View
38 lib/sequel_core/adapters/shared/postgres.rb
@@ -299,6 +299,20 @@ module DatasetMethods
SHARE_ROW_EXCLUSIVE = 'SHARE ROW EXCLUSIVE'.freeze
SHARE_UPDATE_EXCLUSIVE = 'SHARE UPDATE EXCLUSIVE'.freeze
+ # Shared methods for prepared statements when used with PostgreSQL databases.
+ module PreparedStatementMethods
+ # Override insert action to use RETURNING if the server supports it.
+ def prepared_sql
+ return @prepared_sql if @prepared_sql
+ super
+ if @prepared_type == :insert and server_version >= 80200
+ @prepared_sql = insert_returning_pk_sql(@prepared_modify_values)
+ meta_def(:insert_returning_pk_sql){|*args| prepared_sql}
+ end
+ @prepared_sql
+ end
+ end
+
# Return the results of an ANALYZE query as a string
def analyze(opts = nil)
analysis = []
@@ -339,21 +353,21 @@ def full_text_search(cols, terms, opts = {})
# Insert given values into the database.
def insert(*values)
if !@opts[:sql] and server_version >= 80200
- single_value(:sql=>insert_sql(*values))
+ single_value(:sql=>insert_returning_pk_sql(*values))
else
execute_insert(insert_sql(*values), :table=>opts[:from].first,
:values=>values.size == 1 ? values.first : values)
end
end
- # Use the RETURNING clause if not using custom SQL and the server supports it.
- def insert_sql(*values)
- sql = super
- if !@opts[:sql] and server_version >= 80200
- pk = db.primary_key(opts[:from].first)
- sql << " RETURNING #{pk ? quote_identifier(pk) : :NULL}"
- end
- sql
+ # Use the RETURNING clause to return the columns listed in returning.
+ def insert_returning_sql(returning, *values)
+ "#{insert_sql(*values)} RETURNING #{column_list(Array(returning))}"
+ end
+
+ # Insert a record returning the record inserted
+ def insert_select(*values)
+ single_record(:naked=>true, :sql=>insert_returning_sql(nil, *values)) if server_version >= 80200
end
# Handle microseconds for Time and DateTime values, as well as PostgreSQL
@@ -426,6 +440,12 @@ def execute_insert(sql, opts={})
@db.execute_insert(sql, {:server=>@opts[:server] || :default}.merge(opts))
end
+ # Use the RETURNING clause to return the primary key of the inserted record, if it exists
+ def insert_returning_pk_sql(*values)
+ pk = db.primary_key(opts[:from].first)
+ insert_returning_sql(pk ? Sequel::SQL::Identifier.new(pk) : 'NULL'.lit, *values)
+ end
+
# The version of the database server
def server_version
db.server_version(@opts[:server])
View
2  lib/sequel_core/database.rb
@@ -504,7 +504,7 @@ def raise_error(exception, opts={})
e.set_backtrace(exception.backtrace)
raise e
else
- raise
+ raise exception
end
end
View
2  lib/sequel_core/dataset/convenience.rb
@@ -184,7 +184,7 @@ def single_record(opts = nil)
# Returns the first value of the first record in the dataset.
# Returns nil if dataset is empty.
def single_value(opts = nil)
- if r = naked.single_record((opts||{}).merge(:graph=>false))
+ if r = single_record((opts||{}).merge(:graph=>false, :naked=>true))
r.values.first
end
end
View
24 lib/sequel_model/record.rb
@@ -218,15 +218,21 @@ def save!(*columns)
return save_failure(:save) if before_save == false
if @new
return save_failure(:create) if before_create == false
- iid = model.dataset.insert(@values)
- # if we have a regular primary key and it's not set in @values,
- # we assume it's the last inserted id
- if (pk = primary_key) && !(Array === pk) && !@values[pk]
- @values[pk] = iid
- end
- if pk
- @this = nil # remove memoized this dataset
- refresh
+ ds = model.dataset
+ if ds.respond_to?(:insert_select) and h = ds.insert_select(@values)
+ @values = h
+ @this = nil
+ else
+ iid = ds.insert(@values)
+ # if we have a regular primary key and it's not set in @values,
+ # we assume it's the last inserted id
+ if (pk = primary_key) && !(Array === pk) && !@values[pk]
+ @values[pk] = iid
+ end
+ if pk
+ @this = nil # remove memoized this dataset
+ refresh
+ end
end
@new = false
after_create
View
27 spec/adapters/postgres_spec.rb
@@ -450,14 +450,33 @@ def POSTGRES_DB.ret_commit
@ds.delete
end
- specify "should not using RETURNING primary_key if server_version < 80200" do
+ specify "should call insert_sql if server_version < 80200" do
@ds.meta_def(:server_version){80100}
- @ds.insert_sql(:value=>10).should == 'INSERT INTO test5 (value) VALUES (10)'
+ @ds.should_receive(:execute_insert).once.with('INSERT INTO test5 (value) VALUES (10)', :table=>:test5, :values=>{:value=>10})
+ @ds.insert(:value=>10)
end
- specify "should using RETURNING primary_key if server_version >= 80200" do
+ specify "should using call insert_returning_sql if server_version >= 80200" do
@ds.meta_def(:server_version){80201}
- @ds.insert_sql(:value=>10).should == 'INSERT INTO test5 (value) VALUES (10) RETURNING xid'
+ @ds.should_receive(:single_value).once.with(:sql=>'INSERT INTO test5 (value) VALUES (10) RETURNING xid')
+ @ds.insert(:value=>10)
+ end
+
+ specify "should have insert_returning_sql use the RETURNING keyword" do
+ @ds.insert_returning_sql(:xid, :value=>10).should == "INSERT INTO test5 (value) VALUES (10) RETURNING xid"
+ @ds.insert_returning_sql('*'.lit, :value=>10).should == "INSERT INTO test5 (value) VALUES (10) RETURNING *"
+ end
+
+ specify "should have insert_select return nil if server_version < 80200" do
+ @ds.meta_def(:server_version){80100}
+ @ds.insert_select(:value=>10).should == nil
+ end
+
+ specify "should have insert_select insert the record and return the inserted record if server_version < 80200" do
+ @ds.meta_def(:server_version){80201}
+ h = @ds.insert_select(:value=>10)
+ h[:value].should == 10
+ @ds.first(:xid=>h[:xid])[:value].should == 10
end
specify "should correctly return the inserted record's primary key value" do
View
17 spec/sequel_core/database_spec.rb
@@ -999,4 +999,19 @@ def dataset
opts = {:host=>1, :database=>2, :servers=>{:server1=>2}}
proc{MockDatabase.new(opts).send(:server_opts, :server1)}.should raise_error(Sequel::Error)
end
-end
+end
+
+context "Database#raise_error" do
+ specify "should reraise if the exception class is not in opts[:classes]" do
+ e = Class.new(StandardError)
+ proc{MockDatabase.new.send(:raise_error, e.new(''), :classes=>[])}.should raise_error(e)
+ end
+
+ specify "should convert the exception to a DatabaseError if the exception class is not in opts[:classes]" do
+ proc{MockDatabase.new.send(:raise_error, Interrupt.new(''), :classes=>[Interrupt])}.should raise_error(Sequel::DatabaseError)
+ end
+
+ specify "should convert the exception to a DatabaseError if opts[:classes] if not present" do
+ proc{MockDatabase.new.send(:raise_error, Interrupt.new(''))}.should raise_error(Sequel::DatabaseError)
+ end
+end
View
13 spec/sequel_model/record_spec.rb
@@ -17,6 +17,19 @@
MODEL_DB.sqls.first.should == "INSERT INTO items (x) VALUES (1)"
end
+ it "should use dataset's insert_select method if present" do
+ ds = @c.dataset = @c.dataset.clone
+ def ds.insert_select(hash)
+ execute("INSERT INTO items (y) VALUES (2)")
+ {:y=>2}
+ end
+ o = @c.new(:x => 1)
+ o.save
+
+ o.values.should == {:y=>2}
+ MODEL_DB.sqls.first.should == "INSERT INTO items (y) VALUES (2)"
+ end
+
it "should update a record for an existing model instance" do
o = @c.load(:id => 3, :x => 1)
o.save
Please sign in to comment.
Something went wrong with that request. Please try again.