Repository: rails/rails
File: activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb (Ruby)

Code before:
        cast_result(internal_execute(...))
      end

      def default_insert_value(column) # :nodoc:
        DEFAULT_INSERT_VALUE
      end

      private
        DEFAULT_INSERT_VALUE = Arel.sql("DEFAULT").freeze
        private_constant :DEFAULT_INSERT_VALUE

        # Lowest level way to execute a query. Doesn't check for illegal writes, doesn't annotate queries, yields a native result object.
        def raw_execute(intent)
          # Handle Arel compilation if needed
          compile_arel_in_intent(intent)

          # Handle SQL preprocessing if needed
          intent.processed_sql ||= preprocess_query(intent.raw_sql) if intent.raw_sql
          intent.type_casted_binds = type_casted_binds(intent.binds)
          log(intent) do |notification_payload|
            intent.notification_payload = notification_payload
            with_raw_connection(allow_retry: intent.allow_retry, materialize_transactions: intent.materialize_transactions) do |conn|
              result = perform_query(conn, intent)
              handle_warnings(result, intent.processed_sql)
              result
            end
          end
        end

        def perform_query(raw_connection, intent)
          raise NotImplementedError
        end

        def handle_warnings(raw_result, sql)
        end

        # Receive a native adapter result object and returns an ActiveRecord::Result object.
        def cast_result(raw_result)
          raise NotImplementedError
        end

        def affected_rows(raw_result)
          raise NotImplementedError
        end

        def preprocess_query(sql)
          if write_query?(sql)
            ensure_writes_are_allowed(sql)
            mark_transaction_written
          end


Review comment (suggestion):
This feels like it should be part of the constructor, or at least of `compile_arel_in_intent`?

I see there are other codepath only calling one of the two, or none of them but I don't understand why.

Code after:
      # Same as raw_execute but returns an ActiveRecord::Result object.
      def raw_exec_query(intent) # :nodoc:
        cast_result(raw_execute(intent))
      end

      # Execute a query and returns an ActiveRecord::Result
      def internal_exec_query(...) # :nodoc:
        cast_result(internal_execute(...))
      end

      def default_insert_value(column) # :nodoc:
        DEFAULT_INSERT_VALUE
      end

      private
        DEFAULT_INSERT_VALUE = Arel.sql("DEFAULT").freeze
        private_constant :DEFAULT_INSERT_VALUE

        # Lowest level way to execute a query. Doesn't check for illegal writes, doesn't annotate queries, yields a native result object.
        def raw_execute(intent)
          # Handle Arel compilation if needed
          compile_arel_in_intent(intent)

          # Handle SQL preprocessing if needed
          intent.processed_sql ||= preprocess_query(intent.raw_sql) if intent.raw_sql
          intent.type_casted_binds = type_casted_binds(intent.binds)
          log(intent) do |notification_payload|
            intent.notification_payload = notification_payload
            with_raw_connection(allow_retry: intent.allow_retry, materialize_transactions: intent.materialize_transactions) do |conn|
              result = perform_query(conn, intent)
              handle_warnings(result, intent.processed_sql)
              result
            end
          end
        end

        def perform_query(raw_connection, intent)
          raise NotImplementedError
        end

        def handle_warnings(raw_result, sql)
        end

        # Receive a native adapter result object and returns an ActiveRecord::Result object.
        def cast_result(raw_result)
          raise NotImplementedError
        end

        def affected_rows(raw_result)
          raise NotImplementedError
        end