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/database_statements.rb

@@ -9,21 +9,6 @@ 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}"

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/postgresql/adapter.rb

@@ -29,6 +29,7 @@
 require 'active_model'
 
 require "arjdbc/abstract/relation_query_attribute_monkey_patch"
+require "arjdbc/abstract/time_value_jruby_compat_monkey_patch"
 
 module ArJdbc
   # Strives to provide Rails built-in PostgreSQL adapter (API) compatibility.
@@ -104,6 +105,8 @@ def configure_connection
         end
       end
 
+      @raw_connection.decode_dates = decode_dates # Copy to java land for performance
+
       reload_type_map
     end
 
@@ -325,6 +328,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
       }
@@ -596,8 +600,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.

lib/arjdbc/postgresql/schema_statements.rb

@@ -6,23 +6,6 @@ module SchemaStatements
       ForeignKeyDefinition = ActiveRecord::ConnectionAdapters::ForeignKeyDefinition
       Utils = ActiveRecord::ConnectionAdapters::PostgreSQL::Utils
 
-      def decode_string_array(value)
-        return value if value.is_a?(Array)
-        _arjdbc_array_parser.parse_pg_array(value)
-      end
-
-      private
-
-      def _arjdbc_array_parser
-        @_arjdbc_array_parser ||= begin
-          obj = Object.new
-          obj.extend(ActiveRecord::ConnectionAdapters::PostgreSQL::ArrayParser)
-          obj
-        end
-      end
-
-      public
-
       def foreign_keys(table_name)
         scope = quoted_scope(table_name)
         fk_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)

src/java/arjdbc/jdbc/JdbcResult.java

@@ -47,6 +47,16 @@ protected JdbcResult(ThreadContext context, RubyClass clazz, RubyJdbcConnection
         processResultSet(context, resultSet);
     }
 
+    // HACK: Needed for postgres to be able to return a sane result type instead of just a RubyFixnum
+    protected JdbcResult(ThreadContext context, RubyClass clazz, RubyJdbcConnection connection) {
+        super(context.runtime, clazz);
+
+        values = newArray(context);
+        this.connection = connection;
+        columnNames = new RubyString[0];
+        columnTypes = new int[0];
+    }
+
     /**
      * Builds a type map for creating the AR::Result, most adapters don't need it
      * @param context which thread this is running on.

src/java/arjdbc/jdbc/RubyJdbcConnection.java

@@ -814,7 +814,7 @@ public IRubyObject execute(final ThreadContext context, final IRubyObject sql) {
                         result = mapToRawResult(context, connection, resultSet, false);
                         resultSet.close();
                     } else {
-                        result = context.runtime.newFixnum(updateCount);
+                        result = mapEmptyExecuteResult(context, updateCount);
                     }
 
                     // Check to see if there is another result set
@@ -867,6 +867,10 @@ protected IRubyObject mapExecuteResult(final ThreadContext context,
         return mapQueryResult(context, connection, resultSet);
     }
 
+    protected IRubyObject mapEmptyExecuteResult(final ThreadContext context, final long updateCount) {
+        return newEmptyResult(context);
+    }
+
     private static String[] createStatementPk(IRubyObject pk) {
         String[] statementPk;
         if (pk instanceof RubyArray) {
@@ -3274,7 +3278,7 @@ protected boolean supportsGeneratedKeys(final Connection connection) throws SQLE
      * @param downCase should column names only be in lower case?
      */
     @SuppressWarnings("unchecked")
-    private IRubyObject mapToRawResult(final ThreadContext context,
+    protected IRubyObject mapToRawResult(final ThreadContext context,
             final Connection connection, final ResultSet resultSet,
             final boolean downCase) throws SQLException {
 

src/java/arjdbc/postgresql/PostgreSQLResult.java

@@ -9,13 +9,7 @@
 import java.sql.Types;
 
 import arjdbc.util.PG;
-import org.jruby.Ruby;
-import org.jruby.RubyArray;
-import org.jruby.RubyClass;
-import org.jruby.RubyHash;
-import org.jruby.RubyModule;
-import org.jruby.RubyNumeric;
-import org.jruby.RubyString;
+import org.jruby.*;
 import org.jruby.anno.JRubyMethod;
 import org.jruby.runtime.Block;
 import org.jruby.runtime.Helpers;
@@ -39,6 +33,9 @@ public class PostgreSQLResult extends JdbcResult {
     // These are needed when generating an AR::Result
     private final ResultSetMetaData resultSetMetaData;
 
+    // An optional number of updated rows
+    private final long cmdTuples;
+
     /********* JRuby compat methods ***********/
 
     static RubyClass createPostgreSQLResultClass(ThreadContext context, RubyClass postgreSQLConnection) {
@@ -64,13 +61,35 @@ static PostgreSQLResult newResult(ThreadContext context,  RubyClass clazz, Postg
         return new PostgreSQLResult(context, clazz, connection, resultSet);
     }
 
+    /**
+     * Generates a new empty PostgreSQLResult object with a given number of updates
+     * @param context current thread context
+     * @param clazz metaclass for this result object
+     * @param updateCount the number of updated items
+     * @return an instantiated result object
+     * @throws SQLException throws!
+     */
+    static PostgreSQLResult newEmptyResult(ThreadContext context, RubyClass clazz, PostgreSQLRubyJdbcConnection connection,
+                                           long updateCount) {
+        return new PostgreSQLResult(context, clazz, connection, updateCount);
+    }
+
     /********* End JRuby compat methods ***********/
 
     private PostgreSQLResult(ThreadContext context, RubyClass clazz, RubyJdbcConnection connection,
                              ResultSet resultSet) throws SQLException {
         super(context, clazz, connection, resultSet);
 
         resultSetMetaData = resultSet.getMetaData();
+        cmdTuples = -1;
+    }
+
+    private PostgreSQLResult(ThreadContext context, RubyClass clazz, RubyJdbcConnection connection,
+                             long updateCount) {
+        super(context, clazz, connection);
+
+        resultSetMetaData = null;
+        cmdTuples = updateCount;
     }
 
     /**
@@ -107,7 +126,10 @@ protected IRubyObject columnTypeMap(final ThreadContext context) throws SQLExcep
                     runtime.newFixnum(mod),
                     name);
 
-            if (!type.isNil()) types.fastASet(name, type);
+            if (!type.isNil()) {
+                types.fastASet(name, type);
+                types.fastASet(runtime.newFixnum(i), type);
+            }
         }
 
         return types;
@@ -258,12 +280,10 @@ public IRubyObject aref(ThreadContext context, IRubyObject rowArg) {
         return resultHash;
     }
 
-    // Note: this is # of commands (insert/update/selects performed) and not number of rows.  In practice,
-    // so far users always just check this as to when it is 0 which ends up being the same as an update/insert
-    // where no rows were affected...so wrong value but the important value will be the same (I do not see
-    // how jdbc can do this).
+    // Note: This is probably not the best implementation,
+    // but it is better than always returning 0 on non-value-returning ops.
     @PG @JRubyMethod(name = {"cmdtuples", "cmd_tuples"})
     public IRubyObject cmdtuples(ThreadContext context) {
-        return values.isEmpty() ? context.runtime.newFixnum(0) : aref(context, context.runtime.newFixnum(0));
+        return cmdTuples != -1 ? context.runtime.newFixnum(cmdTuples) : values.length(context);
     }
 }

src/java/arjdbc/postgresql/PostgreSQLRubyJdbcConnection.java

@@ -111,6 +111,7 @@ public class PostgreSQLRubyJdbcConnection extends arjdbc.jdbc.RubyJdbcConnection
 
     private RubyClass resultClass;
     private RubyHash typeMap = null;
+    private boolean decodeDates = false;
 
     public PostgreSQLRubyJdbcConnection(Ruby runtime, RubyClass metaClass) {
         super(runtime, metaClass);
@@ -148,6 +149,15 @@ protected String buildURL(final ThreadContext context, final IRubyObject url) {
         return DriverWrapper.buildURL(url, Collections.EMPTY_MAP);
     }
 
+    @Override
+    protected Integer jdbcTypeForPrimitiveAttribute(final ThreadContext context,
+                                                    final IRubyObject attribute) throws SQLException {
+        if (attribute instanceof RubyNumeric || attribute instanceof RubyBoolean) {
+            return Types.VARCHAR;
+        }
+        return super.jdbcTypeForPrimitiveAttribute(context, attribute);
+    }
+
     @Override
     protected DriverWrapper newDriverWrapper(final ThreadContext context, final String driver) {
         DriverWrapper driverWrapper = super.newDriverWrapper(context, driver);
@@ -267,6 +277,22 @@ protected PostgreSQLResult mapExecuteResult(final ThreadContext context, final C
         return PostgreSQLResult.newResult(context, resultClass, this, resultSet);
     }
 
+    @Override
+    protected IRubyObject mapEmptyExecuteResult(final ThreadContext context, final long updateCount) {
+        return PostgreSQLResult.newEmptyResult(context, resultClass, this, updateCount);
+    }
+
+    @Override
+    protected IRubyObject mapToRawResult(final ThreadContext context,
+            final Connection connection, final ResultSet resultSet,
+            final boolean downCase) throws SQLException {
+        if (downCase) {
+            return super.mapToRawResult(context, connection, resultSet, true);
+        } else {
+            return mapExecuteResult(context, connection, resultSet);
+        }
+    }
+
     /**
      * Maps a query result set into a <code>ActiveRecord</code> result.
      * @param context
@@ -804,6 +830,10 @@ protected IRubyObject dateToRuby(ThreadContext context, Ruby runtime, ResultSet
         final String value = resultSet.getString(index);
         if (value == null) return context.nil;
 
+        if (!decodeDates) {
+            return RubyString.newUnicodeString(runtime, value);
+        }
+
         final int len = value.length();
         if (len < 10 && value.charAt(len - 1) == 'y') { // infinity / -infinity
             IRubyObject infinity = parseInfinity(context.runtime, value);
@@ -1084,4 +1114,10 @@ public IRubyObject typemap_set(ThreadContext context, IRubyObject mapArg) {
         this.typeMap = (RubyHash) mapArg;
         return mapArg;
     }
+
+    @PG @JRubyMethod(name = "decode_dates=")
+    public IRubyObject setDecodeDates(ThreadContext context, IRubyObject value) {
+        this.decodeDates = value.isTrue();
+        return value;
+    }
 }

test/rails/excludes/mysql2/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb

@@ -0,0 +1 @@
+exclude :test_url_invalid_adapter, "sqlserver is not a built-in adapter, so rails complains when validating the error"

test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb

@@ -0,0 +1 @@
+exclude :test_url_invalid_adapter, "sqlserver is not a built-in adapter, so rails complains when validating the error"

test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/ReaperTest.rb

@@ -1,2 +1,3 @@
 exclude :test_some_time, 'intermittent failures, leaks thread, fires at high frequency'
 exclude :test_connection_pool_starts_reaper, 'intermittent failures, leaks thread, fires at high frequency'
+exclude :test_reaper_works_after_pool_discard, 'deadlocks under JDBC due to high frequency discard'

test/rails/excludes/sqlite3/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb

@@ -0,0 +1 @@
+exclude :test_url_invalid_adapter, "sqlserver is not a built-in adapter, so rails complains when validating the error"

test/rails/excludes/sqlserver/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb

@@ -0,0 +1 @@
+exclude :test_url_invalid_adapter, "sqlserver is not a built-in adapter, so rails complains when validating the error"