Drop trigger based primary key support for Rails 6

It is planned to support identity type based primary key support
in Oracle enhanced adapter 6.
The current implementation of primary key generation is very complicated.
To make it simple, Oracle enhanced adapter 6 will drop
trigger based primary key support.

`rake db:structure:dump` still dumps triggers
if `structure_dump: db_stored_code` is configured
for those who maintain triggers by themselves.

Closes rsim#1643

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