Merge pull request #1669 from yahonda/drop_trigger_based_primary_key_support

Drop trigger based primary key support for Rails 6

Author: yahonda

lib/active_record/connection_adapters/oracle_enhanced/schema_dumper.rb

@@ -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
@@ -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|

lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb

@@ -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)

lib/active_record/connection_adapters/oracle_enhanced/schema_statements_ext.rb

@@ -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"
@@ -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
@@ -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
@@ -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
 
@@ -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)
 

lib/active_record/connection_adapters/oracle_enhanced_adapter.rb

@@ -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

spec/active_record/connection_adapters/oracle_enhanced/schema_dumper_spec.rb

@@ -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 = {})
@@ -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