Rails 8.0 Support, Postgres fixes

Gemfile

@@ -105,5 +105,6 @@ group :rails do
     gem 'minitest-rg', require: nil
     gem 'minitest-rails', require: nil
     gem 'benchmark-ips', require: nil
+    gem 'rdoc', '~> 7', require: false
   end
 end

activerecord-jdbc-adapter.gemspec

@@ -41,7 +41,7 @@ Gem::Specification.new do |gem|
   gem.executables = gem.files.grep(%r{^bin/}).map { |f| File.basename(f) }
   gem.test_files = gem.files.grep(%r{^test/})
 
-  gem.add_dependency "activerecord", "~> 8.0"
+  gem.add_dependency "activerecord", "~> 8.0.0"
 
   #gem.add_development_dependency 'test-unit', '2.5.4'
   #gem.add_development_dependency 'test-unit-context', '>= 0.3.0'

lib/arjdbc/abstract/core.rb

@@ -57,22 +57,6 @@ def translate_exception(exception, message:, sql:, binds:)
         end
       end
 
-      # this version of log() automatically fills type_casted_binds from binds if necessary
-      def log(sql, name = "SQL", binds = [], type_casted_binds = [], async: false, &block)
-        if binds.any? && (type_casted_binds.nil? || type_casted_binds.empty?)
-          type_casted_binds = lambda {
-            # extract_raw_bind_values
-            binds.map do |bind|
-              if bind.respond_to?(:value_for_database)
-                bind.value_for_database
-              else
-                bind
-              end
-            end
-          }
-        end
-        super
-      end
     end
   end
 

lib/arjdbc/abstract/database_statements.rb

@@ -9,36 +9,18 @@ module DatabaseStatements
 
       NO_BINDS = [].freeze
 
-      unless method_defined?(:mark_transaction_written_if_write)
-        def mark_transaction_written_if_write(sql)
-          if write_query?(sql)
-            ensure_writes_are_allowed(sql)
-            mark_transaction_written
-          end
-        end
-      end
-
-      unless method_defined?(:check_if_write_query)
-        def check_if_write_query(sql)
-          ensure_writes_are_allowed(sql) if write_query?(sql)
-        end
-      end
-
       def exec_insert(sql, name = nil, binds = NO_BINDS, pk = nil, sequence_name = nil, returning: nil)
-        if preventing_writes?
-          raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}"
-        end
-
-        mark_transaction_written_if_write(sql)
+        sql = preprocess_query(sql)
 
         binds = convert_legacy_binds_to_attributes(binds) if binds.first.is_a?(Array)
 
         with_raw_connection do |conn|
           if without_prepared_statement?(binds)
             log(sql, name) { conn.execute_insert_pk(sql, pk) }
           else
-            log(sql, name, binds) do
-              conn.execute_insert_pk(sql, binds, pk)
+            type_casted_binds = type_casted_binds(binds)
+            log(sql, name, binds, type_casted_binds) do
+              conn.execute_insert_pk(sql, type_casted_binds, pk)
             end
           end
         end
@@ -47,41 +29,42 @@ def exec_insert(sql, name = nil, binds = NO_BINDS, pk = nil, sequence_name = nil
       # It appears that at this point (AR 5.0) "prepare" should only ever be true
       # if prepared statements are enabled
       def internal_exec_query(sql, name = nil, binds = NO_BINDS, prepare: false, async: false, allow_retry: false, materialize_transactions: true)
-        if preventing_writes? && write_query?(sql)
-          raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}"
-        end
+        sql = preprocess_query(sql)
 
-        mark_transaction_written_if_write(sql)
+        raw_exec_query(sql, name, binds, prepare: prepare, async: async, allow_retry: allow_retry, materialize_transactions: materialize_transactions)
+      end
 
+      def raw_exec_query(sql, name = nil, binds = NO_BINDS, prepare: false, async: false, allow_retry: false, materialize_transactions: true)
         binds = convert_legacy_binds_to_attributes(binds) if binds.first.is_a?(Array)
 
-        with_raw_connection do |conn|
-          if without_prepared_statement?(binds)
-            log(sql, name, async: async) { conn.execute_query(sql) }
-          else
-            log(sql, name, binds, async: async) do
+        # puts "[1]internal----->sql: #{sql}, binds: #{binds}"
+        type_casted_binds = type_casted_binds(binds)
+        # puts "[2]internal----->sql: #{type_casted_binds.size}, binds: #{type_casted_binds}"
+
+        log(sql, name, binds, type_casted_binds, async: async) do
+          with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|
+            if without_prepared_statement?(binds)
+                conn.execute_query(sql)
+            else
               # this is different from normal AR that always caches
               cached_statement = fetch_cached_statement(sql) if prepare && @jdbc_statement_cache_enabled
-              conn.execute_prepared_query(sql, binds, cached_statement)
+              conn.execute_prepared_query(sql, type_casted_binds, cached_statement)
             end
           end
         end
       end
 
       def exec_update(sql, name = 'SQL', binds = NO_BINDS)
-        if preventing_writes?
-          raise ActiveRecord::ReadOnlyError, "Write query attempted while in readonly mode: #{sql}"
-        end
-
-        mark_transaction_written_if_write(sql)
+        sql = preprocess_query(sql)
 
         binds = convert_legacy_binds_to_attributes(binds) if binds.first.is_a?(Array)
 
         with_raw_connection do |conn|
           if without_prepared_statement?(binds)
             log(sql, name) { conn.execute_update(sql) }
           else
-            log(sql, name, binds) { conn.execute_prepared_update(sql, binds) }
+            type_casted_binds = type_casted_binds(binds)
+            log(sql, name, binds, type_casted_binds) { conn.execute_prepared_update(sql, type_casted_binds) }
           end
         end
       end
@@ -105,12 +88,6 @@ def convert_legacy_binds_to_attributes(binds)
         end
       end
 
-      def preprocess_query(sql)
-        check_if_write_query(sql) if respond_to?(:check_if_write_query, true)
-        mark_transaction_written_if_write(sql) if respond_to?(:mark_transaction_written_if_write, true)
-        sql
-      end
-
       def raw_execute(sql, name, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false)
         log(sql, name, async: async) do
           with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|

lib/arjdbc/abstract/mock_logger_jruby_compat_monkey_patch.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require "active_support/log_subscriber/test_helper"
+
+# As of prism 1.9.0 (possibly earlier), a polyfill exists to handle logging warnings for ruby impls that lack `category:`.
+# As a result of this injection being a public method (as opposed to CRuby's private version),
+# on at least JRuby 10.0.5.0, MockLogger#method_missing never gets hit, which breaks a number of tests.
+# This monkeypatch adds support to the MockLogger to properly capture these log events in spite of this difference.
+module ActiveSupport
+  class LogSubscriber
+    module TestHelper
+      class MockLogger
+        module JRubyCompat
+          ActiveSupport::Logger::Severity.constants.each do |severity|
+            level = severity.downcase
+            define_method(level) do |message = nil, &block|
+              @logged[level] << (block ? block.call : message)
+            end
+          end
+        end
+        prepend JRubyCompat
+      end
+    end
+  end
+end

lib/arjdbc/abstract/time_value_jruby_compat_monkey_patch.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require "active_model/type/helpers/time_value"
+
+# The caller only handles ArgumentError as a failure,
+# but JRuby raises TypeError for invalid formats in Time.new() (non-standard),
+# which isn't handled by the caller.
+# We just return nil here as it will result in fallback parsing, same as raising.
+module ActiveModel
+  module Type
+    module Helpers
+      module TimeValue
+        module JRubyCompat
+          private
+            def fast_string_to_time(string)
+              super
+            rescue TypeError
+              nil
+            end
+        end
+        prepend JRubyCompat
+      end
+    end
+  end
+end

lib/arjdbc/mysql/adapter.rb

@@ -14,6 +14,7 @@
 require "arjdbc/mysql/adapter_hash_config"
 
 require "arjdbc/abstract/relation_query_attribute_monkey_patch"
+require "arjdbc/abstract/mock_logger_jruby_compat_monkey_patch"
 
 module ActiveRecord
   module ConnectionAdapters

lib/arjdbc/postgresql/adapter.rb

@@ -29,6 +29,8 @@
 require 'active_model'
 
 require "arjdbc/abstract/relation_query_attribute_monkey_patch"
+require "arjdbc/abstract/time_value_jruby_compat_monkey_patch"
+require "arjdbc/abstract/mock_logger_jruby_compat_monkey_patch"
 
 module ArJdbc
   # Strives to provide Rails built-in PostgreSQL adapter (API) compatibility.
@@ -104,6 +106,8 @@ def configure_connection
         end
       end
 
+      @raw_connection.decode_dates = decode_dates # Copy to java land for performance
+
       reload_type_map
     end
 
@@ -325,6 +329,7 @@ def enable_extension(name, **)
     #   Set to +:cascade+ to drop dependent objects as well.
     #   Defaults to false.
     def disable_extension(name, force: false)
+      _schema, name = name.to_s.split(".").values_at(-2, -1)
       internal_exec_query("DROP EXTENSION IF EXISTS \"#{name}\"#{' CASCADE' if force == :cascade}").tap {
         reload_type_map
       }
@@ -405,23 +410,27 @@ def drop_enum(name, values = nil, **options)
     end
 
     # Rename an existing enum type to something else.
-    def rename_enum(name, options = {})
-      to = options.fetch(:to) { raise ArgumentError, ":to is required" }
+    def rename_enum(name, new_name = nil, **options)
+      new_name ||= options.fetch(:to) do
+        raise ArgumentError, "rename_enum requires two from/to name positional arguments."
+      end
 
-      exec_query("ALTER TYPE #{quote_table_name(name)} RENAME TO #{to}").tap { reload_type_map }
+      exec_query("ALTER TYPE #{quote_table_name(name)} RENAME TO #{quote_table_name(new_name)}").tap { reload_type_map }
     end
 
     # Add enum value to an existing enum type.
     def add_enum_value(type_name, value, options = {})
       before, after = options.values_at(:before, :after)
-      sql = +"ALTER TYPE #{quote_table_name(type_name)} ADD VALUE '#{value}'"
+      sql = +"ALTER TYPE #{quote_table_name(type_name)} ADD VALUE"
+      sql << " IF NOT EXISTS" if options[:if_not_exists]
+      sql << " #{quote(value)}"
 
       if before && after
         raise ArgumentError, "Cannot have both :before and :after at the same time"
       elsif before
-        sql << " BEFORE '#{before}'"
+        sql << " BEFORE #{quote(before)}"
       elsif after
-        sql << " AFTER '#{after}'"
+        sql << " AFTER #{quote(after)}"
       end
 
       execute(sql).tap { reload_type_map }
@@ -500,8 +509,12 @@ def build_insert_sql(insert) # :nodoc:
         sql << " ON CONFLICT #{insert.conflict_target} DO NOTHING"
       elsif insert.update_duplicates?
         sql << " ON CONFLICT #{insert.conflict_target} DO UPDATE SET "
-        sql << insert.touch_model_timestamps_unless { |column| "#{insert.model.quoted_table_name}.#{column} IS NOT DISTINCT FROM excluded.#{column}" }
-        sql << insert.updatable_columns.map { |column| "#{column}=excluded.#{column}" }.join(",")
+        if insert.raw_update_sql?
+          sql << insert.raw_update_sql
+        else
+          sql << insert.touch_model_timestamps_unless { |column| "#{insert.model.quoted_table_name}.#{column} IS NOT DISTINCT FROM excluded.#{column}" }
+          sql << insert.updatable_columns.map { |column| "#{column}=excluded.#{column}" }.join(",")
+        end
       end
 
       sql << " RETURNING #{insert.returning}" if insert.returning
@@ -515,16 +528,20 @@ def check_version # :nodoc:
     end
 
     def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil, returning: nil) # :nodoc:
-      val = super
-      if !use_insert_returning? && pk
+      if use_insert_returning? || pk == false
+        sql, binds = sql_for_insert(sql, pk, binds, returning)
+        internal_exec_query(sql, name, binds)
+      else
+        result = internal_exec_query(sql, name, binds)
         unless sequence_name
           table_ref = extract_table_ref_from_insert_sql(sql)
-          sequence_name = default_sequence_name(table_ref, pk)
-          return val unless sequence_name
+          if table_ref
+            pk = primary_key(table_ref) if pk.nil?
+            sequence_name = default_sequence_name(table_ref, pk)
+          end
+          return result unless sequence_name
         end
         last_insert_id_result(sequence_name)
-      else
-        val
       end
     end
 
@@ -575,12 +592,6 @@ def disconnect!
       end
     end
 
-    def default_sequence_name(table_name, pk = "id") #:nodoc:
-      serial_sequence(table_name, pk)
-    rescue ActiveRecord::StatementInvalid
-      %Q("#{table_name}_#{pk}_seq")
-    end
-
     def last_insert_id_result(sequence_name)
       exec_query("SELECT currval('#{sequence_name}')", 'SQL')
     end
@@ -596,8 +607,7 @@ def all_schemas
     # Returns the current client message level.
     def client_min_messages
       return nil if redshift? # not supported on Redshift
-      # Need to use #execute so we don't try to access the type map before it is initialized
-      execute('SHOW client_min_messages', 'SCHEMA').values.first.first
+      query_value("SHOW client_min_messages", "SCHEMA")
     end
 
     # Set the client message level.
@@ -706,11 +716,6 @@ def column_definitions(table_name)
       SQL
     end
 
-    def extract_table_ref_from_insert_sql(sql)
-      sql[/into\s("[A-Za-z0-9_."\[\]\s]+"|[A-Za-z0-9_."\[\]]+)\s*/im]
-      $1.strip if $1
-    end
-
     def arel_visitor
       Arel::Visitors::PostgreSQL.new(self)
     end

lib/arjdbc/postgresql/adapter_hash_config.rb

@@ -92,6 +92,9 @@ def build_properties(config)
         properties["prepareThreshold"] = 0
       end
 
+      # Match upstream default PG string type, otherwise incorrectly defaults to varchar
+      properties["stringtype"] ||= "unspecified"
+
       properties
     end
   end

lib/arjdbc/postgresql/connection_methods.rb

@@ -63,6 +63,9 @@ def postgresql_connection(config)
       properties['prepareThreshold'] = 0
     end
 
+    # Match upstream default PG string type, otherwise incorrectly defaults to varchar
+    properties['stringtype'] ||= 'unspecified'
+
     jdbc_connection(config)
   end
   alias_method :jdbcpostgresql_connection, :postgresql_connection