Skip to content

Drop trigger based primary key support for Rails 6 #1669

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 8, 2018
Merged

Drop trigger based primary key support for Rails 6 #1669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions lib/active_record/connection_adapters/oracle_enhanced/schema_dumper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ def tables(stream)
# add table prefix or suffix for schema_migrations
next if ignored? tbl
table(tbl, stream)
# add primary key trigger if table has it
primary_key_trigger(tbl, stream)
end
# following table definitions
# add foreign keys if table has them
Expand All @@ -27,15 +25,6 @@ def tables(stream)
synonyms(stream)
end

def primary_key_trigger(table_name, stream)
if @connection.has_primary_key_trigger?(table_name)
pk, _pk_seq = @connection.pk_and_sequence_for(table_name)
stream.print " add_primary_key_trigger #{table_name.inspect}"
stream.print ", primary_key: \"#{pk}\"" if pk != "id"
stream.print "\n\n"
end
end

def synonyms(stream)
syns = @connection.synonyms
syns.each do |syn|
Expand Down
26 changes: 2 additions & 24 deletions lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,33 +658,11 @@ def column_for(table_name, column_name)
end

def create_sequence_and_trigger(table_name, options)
# TODO: Needs rename since no triggers created
# This method will be removed since sequence will not be created separately
seq_name = options[:sequence_name] || default_sequence_name(table_name)
seq_start_value = options[:sequence_start_value] || default_sequence_start_value
execute "CREATE SEQUENCE #{quote_table_name(seq_name)} START WITH #{seq_start_value}"

create_primary_key_trigger(table_name, options) if options[:primary_key_trigger]
end

def create_primary_key_trigger(table_name, options)
seq_name = options[:sequence_name] || default_sequence_name(table_name)
trigger_name = options[:trigger_name] || default_trigger_name(table_name)
primary_key = options[:primary_key] || Base.get_primary_key(table_name.to_s.singularize)
execute <<-SQL
CREATE OR REPLACE TRIGGER #{quote_table_name(trigger_name)}
BEFORE INSERT ON #{quote_table_name(table_name)} FOR EACH ROW
BEGIN
IF inserting THEN
IF :new.#{quote_column_name(primary_key)} IS NULL THEN
SELECT #{quote_table_name(seq_name)}.NEXTVAL INTO :new.#{quote_column_name(primary_key)} FROM dual;
END IF;
END IF;
END;
SQL
end

def default_trigger_name(table_name)
# truncate table name if necessary to fit in max length of identifier
"#{table_name.to_s[0, table_name_length - 4]}_pkt"
end

def rebuild_primary_key_index_to_default_tablespace(table_name, options)
Expand Down
28 changes: 0 additions & 28 deletions lib/active_record/connection_adapters/oracle_enhanced/schema_statements_ext.rb
View file
Open in desktop

This file was deleted.

23 changes: 2 additions & 21 deletions lib/active_record/connection_adapters/oracle_enhanced_adapter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
require "active_record/connection_adapters/oracle_enhanced/schema_definitions"
require "active_record/connection_adapters/oracle_enhanced/schema_dumper"
require "active_record/connection_adapters/oracle_enhanced/schema_statements"
require "active_record/connection_adapters/oracle_enhanced/schema_statements_ext"
require "active_record/connection_adapters/oracle_enhanced/context_index"
require "active_record/connection_adapters/oracle_enhanced/column"
require "active_record/connection_adapters/oracle_enhanced/quoting"
Expand Down Expand Up @@ -142,7 +141,6 @@ module ConnectionAdapters #:nodoc:
class OracleEnhancedAdapter < AbstractAdapter
include OracleEnhanced::DatabaseStatements
include OracleEnhanced::SchemaStatements
include OracleEnhanced::SchemaStatementsExt
include OracleEnhanced::ContextIndex
include OracleEnhanced::Quoting
include OracleEnhanced::DatabaseLimits
Expand Down Expand Up @@ -415,7 +413,7 @@ def discard!
# when inserting a new database record (see #prefetch_primary_key?).
def next_sequence_value(sequence_name)
# if sequence_name is set to :autogenerated then it means that primary key will be populated by trigger
return nil if sequence_name == AUTOGENERATED_SEQUENCE_NAME
raise ArgumentError "Trigger based primary key is not supported" if sequence_name == AUTOGENERATED_SEQUENCE_NAME
# call directly connection method to avoid prepared statement which causes fetching of next sequence value twice
select_value(<<-SQL.strip.gsub(/\s+/, " "), "next sequence value")
SELECT #{quote_table_name(sequence_name)}.NEXTVAL FROM dual
Expand All @@ -428,7 +426,7 @@ def prefetch_primary_key?(table_name = nil)
return true if table_name.nil?
table_name = table_name.to_s
owner, desc_table_name = @connection.describe(table_name)
do_not_prefetch = !has_primary_key?(table_name, owner, desc_table_name) || has_primary_key_trigger?(table_name, owner, desc_table_name)
do_not_prefetch = !has_primary_key?(table_name, owner, desc_table_name)
!do_not_prefetch
end

Expand Down Expand Up @@ -494,23 +492,6 @@ def default_tablespace
SQL
end

# check if table has primary key trigger with _pkt suffix
def has_primary_key_trigger?(table_name, owner = nil, desc_table_name = nil)
(owner, desc_table_name) = @connection.describe(table_name) unless owner

trigger_name = default_trigger_name(table_name).upcase

!!select_value(<<-SQL.strip.gsub(/\s+/, " "), "Primary Key Trigger", [bind_string("owner", owner), bind_string("trigger_name", trigger_name), bind_string("owner", owner), bind_string("table_name", desc_table_name)])
SELECT trigger_name
FROM all_triggers
WHERE owner = :owner
AND trigger_name = :trigger_name
AND table_owner = :owner
AND table_name = :table_name
AND status = 'ENABLED'
SQL
end

def column_definitions(table_name)
(owner, desc_table_name) = @connection.describe(table_name)

Expand Down
18 changes: 0 additions & 18 deletions spec/active_record/connection_adapters/oracle_enhanced/schema_dumper_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,24 +110,6 @@ def drop_test_posts_table
end
end

describe "table with primary key trigger" do

after(:each) do
drop_test_posts_table
end

it "should include primary key trigger in schema dump" do
create_test_posts_table(primary_key_trigger: true)
expect(standard_dump).to match(/create_table "test_posts".*add_primary_key_trigger "test_posts"/m)
end

it "should include primary key trigger with non-default primary key in schema dump" do
create_test_posts_table(primary_key_trigger: true, primary_key: "post_id")
expect(standard_dump).to match(/create_table "test_posts", primary_key: "post_id".*add_primary_key_trigger "test_posts", primary_key: "post_id"/m)
end

end

describe "foreign key constraints" do
before(:all) do
schema_define do
Expand Down
204 changes: 0 additions & 204 deletions spec/active_record/connection_adapters/oracle_enhanced/schema_statements_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,172 +153,6 @@ class ::TestEmployee < ActiveRecord::Base; end

end

describe "create table with primary key trigger" do
def create_table_with_trigger(options = {})
options.merge! primary_key_trigger: true, force: true
schema_define do
create_table :test_employees, options do |t|
t.string :first_name
t.string :last_name
end
end
end

def create_table_and_separately_trigger(options = {})
options.merge! force: true
schema_define do
create_table :test_employees, options do |t|
t.string :first_name
t.string :last_name
end
add_primary_key_trigger :test_employees, options
end
end

def drop_table_with_trigger(options = {})
seq_name = options[:sequence_name]
schema_define do
drop_table :test_employees, (seq_name ? { sequence_name: seq_name } : {})
end
Object.send(:remove_const, "TestEmployee")
ActiveRecord::Base.clear_cache!
end

describe "with default primary key" do
before(:all) do
@conn = ActiveRecord::Base.connection
create_table_with_trigger
class ::TestEmployee < ActiveRecord::Base
end
end

after(:all) do
drop_table_with_trigger
end

it "should populate primary key using trigger" do
expect do
@conn.execute "INSERT INTO test_employees (first_name) VALUES ('Raimonds')"
end.not_to raise_error
end

it "should return new key value using connection insert method" do
insert_id = @conn.insert("INSERT INTO test_employees (first_name) VALUES ('Raimonds')", nil, "id")
expect(@conn.select_value("SELECT test_employees_seq.currval FROM dual")).to eq(insert_id)
end

it "should create new record for model" do
e = TestEmployee.create!(first_name: "Raimonds")
expect(@conn.select_value("SELECT test_employees_seq.currval FROM dual")).to eq(e.id)
end

it "should not generate NoMethodError for :returning_id:Symbol" do
set_logger
@conn.reconnect! unless @conn.active?
@conn.insert("INSERT INTO test_employees (first_name) VALUES ('Yasuo')", nil, "id")
expect(@logger.output(:error)).not_to match(/^Could not log "sql.active_record" event. NoMethodError: undefined method `name' for :returning_id:Symbol/)
clear_logger
end

end

describe "with separate creation of primary key trigger" do
before(:all) do
ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
@conn = ActiveRecord::Base.connection
create_table_and_separately_trigger
class ::TestEmployee < ActiveRecord::Base
end
end

after(:all) do
drop_table_with_trigger
end

it "should populate primary key using trigger" do
expect do
@conn.execute "INSERT INTO test_employees (first_name) VALUES ('Raimonds')"
end.not_to raise_error
end

it "should return new key value using connection insert method" do
insert_id = @conn.insert("INSERT INTO test_employees (first_name) VALUES ('Raimonds')", nil, "id")
expect(@conn.select_value("SELECT test_employees_seq.currval FROM dual")).to eq(insert_id)
end

it "should create new record for model" do
e = TestEmployee.create!(first_name: "Raimonds")
expect(@conn.select_value("SELECT test_employees_seq.currval FROM dual")).to eq(e.id)
end
end

describe "with non-default primary key and non-default sequence name" do
before(:all) do
ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
@conn = ActiveRecord::Base.connection
@primary_key = "employee_id"
@sequence_name = "test_employees_s"
create_table_with_trigger(primary_key: @primary_key, sequence_name: @sequence_name)
class ::TestEmployee < ActiveRecord::Base
self.primary_key = "employee_id"
end
end

after(:all) do
drop_table_with_trigger(sequence_name: @sequence_name)
end

it "should populate primary key using trigger" do
expect do
@conn.execute "INSERT INTO test_employees (first_name) VALUES ('Raimonds')"
end.not_to raise_error
end

it "should return new key value using connection insert method" do
insert_id = @conn.insert("INSERT INTO test_employees (first_name) VALUES ('Raimonds')", nil, @primary_key)
expect(@conn.select_value("SELECT #{@sequence_name}.currval FROM dual")).to eq(insert_id)
end

it "should create new record for model with autogenerated sequence option" do
e = TestEmployee.create!(first_name: "Raimonds")
expect(@conn.select_value("SELECT #{@sequence_name}.currval FROM dual")).to eq(e.id)
end
end

describe "with non-default sequence name and non-default trigger name" do
before(:all) do
ActiveRecord::Base.establish_connection(CONNECTION_PARAMS)
@conn = ActiveRecord::Base.connection
@sequence_name = "test_employees_s"
create_table_with_trigger(sequence_name: @sequence_name, trigger_name: "test_employees_t1")
class ::TestEmployee < ActiveRecord::Base
self.sequence_name = :autogenerated
end
end

after(:all) do
drop_table_with_trigger(sequence_name: @sequence_name)
end

it "should populate primary key using trigger" do
expect do
@conn.execute "INSERT INTO test_employees (first_name) VALUES ('Raimonds')"
end.not_to raise_error
end

it "should return new key value using connection insert method" do
insert_id = @conn.insert("INSERT INTO test_employees (first_name) VALUES ('Raimonds')", nil, "id")
expect(@conn.select_value("SELECT #{@sequence_name}.currval FROM dual")).to eq(insert_id)
end

it "should create new record for model with autogenerated sequence option" do
e = TestEmployee.create!(first_name: "Raimonds")
expect(@conn.select_value("SELECT #{@sequence_name}.currval FROM dual")).to eq(e.id)
end
end

end

describe "table and column comments" do

def create_test_employees_table(table_comment = nil, column_comments = {})
Expand Down Expand Up @@ -474,44 +308,6 @@ class ::TestEmployee < ActiveRecord::Base; end

end

describe "create triggers" do

before(:all) do
@conn = ActiveRecord::Base.connection
schema_define do
create_table :test_employees do |t|
t.string :first_name
t.string :last_name
end
end
class ::TestEmployee < ActiveRecord::Base; end
end

after(:all) do
schema_define do
drop_table :test_employees
end
Object.send(:remove_const, "TestEmployee")
ActiveRecord::Base.clear_cache!
end

it "should create table trigger with :new reference" do
expect do
@conn.execute <<-SQL
CREATE OR REPLACE TRIGGER test_employees_pkt
BEFORE INSERT ON test_employees FOR EACH ROW
BEGIN
IF inserting THEN
IF :new.id IS NULL THEN
SELECT test_employees_seq.NEXTVAL INTO :new.id FROM dual;
END IF;
END IF;
END;
SQL
end.not_to raise_error
end
end

describe "add index" do
before(:all) do
@conn = ActiveRecord::Base.connection
Expand Down
Loading