diff --git a/Gemfile b/Gemfile index 409024dd0..8f20f9d3f 100644 --- a/Gemfile +++ b/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 diff --git a/activerecord-jdbc-adapter.gemspec b/activerecord-jdbc-adapter.gemspec index f12115079..a32506441 100644 --- a/activerecord-jdbc-adapter.gemspec +++ b/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' diff --git a/lib/arjdbc/abstract/core.rb b/lib/arjdbc/abstract/core.rb index 2ed5d8c63..7806c5d6e 100644 --- a/lib/arjdbc/abstract/core.rb +++ b/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 diff --git a/lib/arjdbc/abstract/database_statements.rb b/lib/arjdbc/abstract/database_statements.rb index 856619a23..0f88963f8 100644 --- a/lib/arjdbc/abstract/database_statements.rb +++ b/lib/arjdbc/abstract/database_statements.rb @@ -9,27 +9,8 @@ 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) @@ -37,8 +18,9 @@ def exec_insert(sql, name = nil, binds = NO_BINDS, pk = nil, sequence_name = nil 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,33 +29,33 @@ 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) @@ -81,7 +63,8 @@ def exec_update(sql, name = 'SQL', binds = NO_BINDS) 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| diff --git a/lib/arjdbc/abstract/mock_logger_jruby_compat_monkey_patch.rb b/lib/arjdbc/abstract/mock_logger_jruby_compat_monkey_patch.rb new file mode 100644 index 000000000..3af511f2b --- /dev/null +++ b/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 diff --git a/lib/arjdbc/abstract/time_value_jruby_compat_monkey_patch.rb b/lib/arjdbc/abstract/time_value_jruby_compat_monkey_patch.rb new file mode 100644 index 000000000..2d6e40424 --- /dev/null +++ b/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 diff --git a/lib/arjdbc/mysql/adapter.rb b/lib/arjdbc/mysql/adapter.rb index 276205b00..06eab0d14 100644 --- a/lib/arjdbc/mysql/adapter.rb +++ b/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 diff --git a/lib/arjdbc/postgresql/adapter.rb b/lib/arjdbc/postgresql/adapter.rb index 88adbb878..9a3400732 100644 --- a/lib/arjdbc/postgresql/adapter.rb +++ b/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 diff --git a/lib/arjdbc/postgresql/adapter_hash_config.rb b/lib/arjdbc/postgresql/adapter_hash_config.rb index bc042ac1f..413321831 100644 --- a/lib/arjdbc/postgresql/adapter_hash_config.rb +++ b/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 diff --git a/lib/arjdbc/postgresql/connection_methods.rb b/lib/arjdbc/postgresql/connection_methods.rb index a1b5d7441..4a7190d9c 100644 --- a/lib/arjdbc/postgresql/connection_methods.rb +++ b/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 diff --git a/lib/arjdbc/postgresql/database_statements.rb b/lib/arjdbc/postgresql/database_statements.rb index 2c1ddc85e..5d66db1b3 100644 --- a/lib/arjdbc/postgresql/database_statements.rb +++ b/lib/arjdbc/postgresql/database_statements.rb @@ -15,6 +15,33 @@ def build_explain_clause(options = []) "EXPLAIN (#{options.join(", ").upcase})" end + + # Set when constraints will be checked for the current transaction. + # + # Not passing any specific constraint names will set the value for all deferrable constraints. + # + # [deferred] + # Valid values are +:deferred+ or +:immediate+. + # + # See https://www.postgresql.org/docs/current/sql-set-constraints.html + def set_constraints(deferred, *constraints) + unless %i[deferred immediate].include?(deferred) + raise ArgumentError, "deferred must be :deferred or :immediate" + end + + constraints = if constraints.empty? + "ALL" + else + constraints.map { |c| quote_table_name(c) }.join(", ") + end + execute("SET CONSTRAINTS #{constraints} #{deferred.to_s.upcase}") + end + + private + + def returning_column_values(result) + result.rows.first + end end end end diff --git a/lib/arjdbc/postgresql/oid_types.rb b/lib/arjdbc/postgresql/oid_types.rb index cd6e6cff9..8499d2203 100644 --- a/lib/arjdbc/postgresql/oid_types.rb +++ b/lib/arjdbc/postgresql/oid_types.rb @@ -147,8 +147,8 @@ def initialize_type_map_inner(m) m.register_type "regproc", OID::Enum.new # FIXME: adding this vector type leads to quoting not handlign Array data in quoting. #m.register_type "_int4", OID::Vector.new(",", m.lookup("int4")) - register_class_with_precision m, "time", Type::Time - register_class_with_precision m, "timestamp", OID::Timestamp + register_class_with_precision m, "time", Type::Time, timezone: @default_timezone + register_class_with_precision m, "timestamp", OID::Timestamp, timezone: @default_timezone register_class_with_precision m, "timestamptz", OID::TimestampWithTimeZone m.register_type "numeric" do |_, fmod, sql_type| diff --git a/lib/arjdbc/postgresql/schema_statements.rb b/lib/arjdbc/postgresql/schema_statements.rb index 50d64bfea..41f3ef91b 100644 --- a/lib/arjdbc/postgresql/schema_statements.rb +++ b/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) diff --git a/lib/arjdbc/sqlite3/adapter.rb b/lib/arjdbc/sqlite3/adapter.rb index b97f73f73..438be21df 100644 --- a/lib/arjdbc/sqlite3/adapter.rb +++ b/lib/arjdbc/sqlite3/adapter.rb @@ -21,6 +21,7 @@ require "arjdbc/sqlite3/pragmas" require "arjdbc/abstract/relation_query_attribute_monkey_patch" +require "arjdbc/abstract/mock_logger_jruby_compat_monkey_patch" module SQLite3 module Constants diff --git a/src/java/arjdbc/jdbc/JdbcResult.java b/src/java/arjdbc/jdbc/JdbcResult.java index 401f36dcc..03dde51b0 100644 --- a/src/java/arjdbc/jdbc/JdbcResult.java +++ b/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. diff --git a/src/java/arjdbc/jdbc/RubyJdbcConnection.java b/src/java/arjdbc/jdbc/RubyJdbcConnection.java index d88d9a8d6..22b10ca7b 100644 --- a/src/java/arjdbc/jdbc/RubyJdbcConnection.java +++ b/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 { diff --git a/src/java/arjdbc/postgresql/PgDateTimeUtils.java b/src/java/arjdbc/postgresql/PgDateTimeUtils.java index 0890b0052..c07d46af1 100644 --- a/src/java/arjdbc/postgresql/PgDateTimeUtils.java +++ b/src/java/arjdbc/postgresql/PgDateTimeUtils.java @@ -3,7 +3,9 @@ import arjdbc.util.DateTimeUtils; import org.joda.time.DateTimeZone; import org.jruby.RubyArray; -import org.jruby.RubyFloat; +import org.jruby.RubyNumeric; +import org.jruby.RubyInteger; +import org.jruby.RubyFixnum; import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; @@ -22,12 +24,18 @@ public abstract class PgDateTimeUtils extends DateTimeUtils { */ public static String timestampValueToString(final ThreadContext context, IRubyObject value, DateTimeZone zone, boolean withZone) { - if (value instanceof RubyFloat) { - final double dv = ((RubyFloat) value).getValue(); - if (dv == Double.POSITIVE_INFINITY) { - return "infinity"; - } else if (dv == Double.NEGATIVE_INFINITY) { - return "-infinity"; + if (value instanceof RubyNumeric valueNumeric) { + final IRubyObject infinite = valueNumeric.infinite_p(context); + if (infinite instanceof RubyFixnum infiniteFixnum) { + if (infiniteFixnum.getValue() > 0) { + return "infinity"; + } else { + return "-infinity"; + } + } + // Rails 8.0 removed Numeric#toTime, so we have to handle this here + if (valueNumeric instanceof RubyInteger) { + return valueNumeric.asString().toString(); } } return timestampTimeToString(context, value, zone, withZone); diff --git a/src/java/arjdbc/postgresql/PostgreSQLResult.java b/src/java/arjdbc/postgresql/PostgreSQLResult.java index 4643db42a..344c1fef9 100644 --- a/src/java/arjdbc/postgresql/PostgreSQLResult.java +++ b/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,6 +61,19 @@ 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, @@ -71,6 +81,15 @@ private PostgreSQLResult(ThreadContext context, RubyClass clazz, RubyJdbcConnect 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); } } diff --git a/src/java/arjdbc/postgresql/PostgreSQLRubyJdbcConnection.java b/src/java/arjdbc/postgresql/PostgreSQLRubyJdbcConnection.java index 51274ce95..3dabcc036 100644 --- a/src/java/arjdbc/postgresql/PostgreSQLRubyJdbcConnection.java +++ b/src/java/arjdbc/postgresql/PostgreSQLRubyJdbcConnection.java @@ -52,6 +52,8 @@ import org.jruby.runtime.ObjectAllocator; import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; +import org.jruby.runtime.callsite.CachingCallSite; +import org.jruby.runtime.callsite.FunctionalCachingCallSite; import org.jruby.util.ByteList; import org.jruby.util.TypeConverter; @@ -111,6 +113,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 +151,17 @@ 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; + } else if (attribute instanceof RubyHash) { // Should be a pg-style bind-param hash + return Types.BINARY; + } + return super.jdbcTypeForPrimitiveAttribute(context, attribute); + } + @Override protected DriverWrapper newDriverWrapper(final ThreadContext context, final String driver) { DriverWrapper driverWrapper = super.newDriverWrapper(context, driver); @@ -267,6 +281,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 ActiveRecord result. * @param context @@ -298,13 +328,32 @@ protected void setArrayParameter(final ThreadContext context, break; } default: - values = valueForDB.toArray(); + final IRubyObject adapter = ActiveRecord(context).getClass(context, "Base").callMethod(context, "connection"); + values = typeCastArrayValues(context, adapter, valueForDB); break; } statement.setArray(index, connection.createArrayOf(typeName, values)); } + private final CachingCallSite type_cast_site = new FunctionalCachingCallSite("type_cast"); + + private Object[] typeCastArrayValues(final ThreadContext context, final IRubyObject adapter, final RubyArray values) { + final int size = values.size(); + final Object[] result = new Object[size]; + for (int i = 0; i < size; i++) { + final IRubyObject elem = values.eltInternal(i); + if (elem instanceof RubyArray arrayElem) { + result[i] = typeCastArrayValues(context, adapter, arrayElem); + } else { + // This could be a performance bottleneck, but it ensures that behaviour aligns with PG gem. Might want to revisit this later. + final IRubyObject cast = type_cast_site.call(context, adapter, adapter, elem); + result[i] = cast.isNil() ? null : cast.toJava(Object.class); + } + } + return result; + } + protected void setDecimalParameter(final ThreadContext context, final Connection connection, final PreparedStatement statement, final int index, final IRubyObject value, @@ -332,8 +381,11 @@ protected void setBlobParameter(final ThreadContext context, if ( value instanceof RubyIO ) { // IO/File statement.setBinaryStream(index, ((RubyIO) value).getInStream()); } - else { // should be a RubyString - final ByteList bytes = value.asString().getByteList(); + else { // should be a RubyString, or pg-style bind-param hash + final IRubyObject binary = value instanceof RubyHash hashValue + ? hashValue.op_aref(context, context.runtime.newSymbol("value")) + : value; + final ByteList bytes = binary.asString().getByteList(); statement.setBinaryStream(index, new ByteArrayInputStream(bytes.unsafeBytes(), bytes.getBegin(), bytes.getRealSize()), bytes.getRealSize() // length @@ -347,7 +399,7 @@ protected void setTimestampParameter(final ThreadContext context, final int index, IRubyObject value, final IRubyObject attribute, final int type) throws SQLException { // PGJDBC uses strings internally anyway, so using Timestamp doesn't do any good - String tsString = PgDateTimeUtils.timestampValueToString(context, value, null, true); + String tsString = PgDateTimeUtils.timestampValueToString(context, value, getDefaultTimeZone(context), true); statement.setObject(index, tsString, Types.OTHER); } @@ -612,10 +664,7 @@ private void setJsonParameter(final ThreadContext context, final PreparedStatement statement, final int index, final IRubyObject value, final String columnType) throws SQLException { - final PGobject pgJson = new PGobject(); - pgJson.setType(columnType); - pgJson.setValue(value.toString()); - statement.setObject(index, pgJson); + statement.setObject(index, value.toString(), Types.OTHER); } private void setPGobjectParameter(final PreparedStatement statement, final int index, @@ -804,6 +853,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); @@ -854,10 +907,9 @@ protected IRubyObject objectToRuby(ThreadContext context, Ruby runtime, ResultSe } if (object instanceof Map) { // hstore - // by default we avoid double parsing by driver and then column : - final RubyHash rubyObject = RubyHash.newHash(context.runtime); - rubyObject.putAll((Map) object); // converts keys/values to ruby - return rubyObject; + // This will be parsed by OID::HStore#deserialize + // Can't use hash as before due to hash breaking dirty tracking + return runtime.newString(resultSet.getString(index)); } return JavaUtil.convertJavaToRuby(runtime, object); @@ -1084,4 +1136,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; + } } diff --git a/test/rails/active_support/callbacks.rb b/test/rails/active_support/callbacks.rb new file mode 100644 index 000000000..82e86f9ef --- /dev/null +++ b/test/rails/active_support/callbacks.rb @@ -0,0 +1,52 @@ +# Partial workaround for JRuby having no GIL - tests like the reaper etc spuriously fail due to multithreaded test execution +# assuming a GIL that does not actually exist for JRuby. We really do love non-deterministic multithreaded execution, +# it never, ever causes random bugs that spuriously fail without any consistent way to reproduce them! +# Multithreading. Not even once. +# Do we want to make this happen both in and out of tests? Might be worth considering. + +# We want to inject *after* this is loaded, so gonna load it first. There's probably a better way to do this. +real = $LOAD_PATH + .map { |dir| File.expand_path(File.join(dir, "active_support", "callbacks.rb")) } + .find { |path| path != File.expand_path(__FILE__) && File.exist?(path) } +require real + +require "monitor" + +module ActiveSupport + module Callbacks + module ClassMethods + REGISTRATION_MONITOR = Monitor.new + + module ThreadSafeRegistration + def set_callback(*, &block) + REGISTRATION_MONITOR.synchronize { super } + end + + def skip_callback(*, &block) + REGISTRATION_MONITOR.synchronize { super } + end + + def reset_callbacks(*) + REGISTRATION_MONITOR.synchronize { super } + end + + def define_callbacks(*) + REGISTRATION_MONITOR.synchronize { super } + end + + protected + # Publish by atomic reference swap rather than the in-place mutation the + # original performs, so unlocked readers never see a torn hash. + def set_callbacks(name, callbacks) + REGISTRATION_MONITOR.synchronize do + new_callbacks = __callbacks.dup + new_callbacks[name.to_sym] = callbacks + self.__callbacks = new_callbacks + end + end + end + + prepend ThreadSafeRegistration + end + end +end diff --git a/test/rails/excludes/mysql2/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb b/test/rails/excludes/mysql2/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb new file mode 100644 index 000000000..ff8ea495d --- /dev/null +++ b/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" diff --git a/test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb b/test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb new file mode 100644 index 000000000..ff8ea495d --- /dev/null +++ b/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" diff --git a/test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/ReaperTest.rb b/test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/ReaperTest.rb index 3f4f5c8e6..46049e4ad 100644 --- a/test/rails/excludes/postgresql/ActiveRecord/ConnectionAdapters/ReaperTest.rb +++ b/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' diff --git a/test/rails/excludes/postgresql/ActiveRecordMessagePackTest.rb b/test/rails/excludes/postgresql/ActiveRecordMessagePackTest.rb new file mode 100644 index 000000000..8b7515aad --- /dev/null +++ b/test/rails/excludes/postgresql/ActiveRecordMessagePackTest.rb @@ -0,0 +1,4 @@ +exclude :test_roundtrips_record_and_cached_associations, 'msgpack-jruby class resolution is busted, gets first valid instead of most applicable' +exclude :"test_roundtrips_new_record?_status", 'msgpack-jruby class resolution is busted, gets first valid instead of most applicable' +exclude :test_roundtrips_binary_attribute, 'msgpack-jruby class resolution is busted, gets first valid instead of most applicable' +exclude :"test_raises_ActiveSupport::MessagePack::MissingClassError_if_record_class_no_longer_exists", 'msgpack-jruby class resolution is busted, gets first valid instead of most applicable' diff --git a/test/rails/excludes/postgresql/AssociationsExtensionsTest.rb b/test/rails/excludes/postgresql/AssociationsExtensionsTest.rb new file mode 100644 index 000000000..66987968f --- /dev/null +++ b/test/rails/excludes/postgresql/AssociationsExtensionsTest.rb @@ -0,0 +1,6 @@ +[ + :test_marshalling_extensions, + :test_marshalling_named_extensions +].each do |name| + exclude name, 'Activerecord 6.1 marshalling format is broken with JRuby 10.0.5.0 (cyclic refs with custom marshal) - AR 7.1 marshal works fine' +end diff --git a/test/rails/excludes/postgresql/BasicsTest.rb b/test/rails/excludes/postgresql/BasicsTest.rb index ea57b1d0b..044a91cc5 100644 --- a/test/rails/excludes/postgresql/BasicsTest.rb +++ b/test/rails/excludes/postgresql/BasicsTest.rb @@ -1,6 +1,14 @@ [ # NOTE: these are copied to AR-JDBC's suite with proper (JVM) TZ adjustment + :test_preserving_time_objects_with_local_time_conversion_to_default_timezone_utc, :test_preserving_time_objects_with_utc_time_conversion_to_default_timezone_local, :test_preserving_time_objects_with_time_with_zone_conversion_to_default_timezone_local ].each do |name| exclude name, 'assuming ENV[TZ] change reflects system (JVM) TimeZone default change' end + +[ + :test_marshalling_with_associations_6_1, + :test_marshalling_new_record_round_trip_with_associations +].each do |name| + exclude name, 'Activerecord 6.1 marshalling format is broken with JRuby 10.0.5.0 (cyclic refs with custom marshal) - AR 7.1 marshal works fine' +end diff --git a/test/rails/excludes/postgresql/HasAndBelongsToManyAssociationsTest.rb b/test/rails/excludes/postgresql/HasAndBelongsToManyAssociationsTest.rb new file mode 100644 index 000000000..1bc1dda33 --- /dev/null +++ b/test/rails/excludes/postgresql/HasAndBelongsToManyAssociationsTest.rb @@ -0,0 +1 @@ +exclude :test_marshal_dump, 'Activerecord 6.1 marshalling format is broken with JRuby 10.0.5.0 (cyclic refs with custom marshal) - AR 7.1 marshal works fine' diff --git a/test/rails/excludes/postgresql/HasManyThroughAssociationsTest.rb b/test/rails/excludes/postgresql/HasManyThroughAssociationsTest.rb new file mode 100644 index 000000000..1bc1dda33 --- /dev/null +++ b/test/rails/excludes/postgresql/HasManyThroughAssociationsTest.rb @@ -0,0 +1 @@ +exclude :test_marshal_dump, 'Activerecord 6.1 marshalling format is broken with JRuby 10.0.5.0 (cyclic refs with custom marshal) - AR 7.1 marshal works fine' diff --git a/test/rails/excludes/postgresql/MarshalSerializationTest.rb b/test/rails/excludes/postgresql/MarshalSerializationTest.rb new file mode 100644 index 000000000..bd9c58230 --- /dev/null +++ b/test/rails/excludes/postgresql/MarshalSerializationTest.rb @@ -0,0 +1 @@ +exclude :test_rails_6_1_rountrip, 'Activerecord 6.1 marshalling format is broken with JRuby 10.0.5.0 (cyclic refs with custom marshal) - AR 7.1 marshal works fine' diff --git a/test/rails/excludes/postgresql/PessimisticLockingTest.rb b/test/rails/excludes/postgresql/PessimisticLockingTest.rb index ad897f04d..3a55a8433 100644 --- a/test/rails/excludes/postgresql/PessimisticLockingTest.rb +++ b/test/rails/excludes/postgresql/PessimisticLockingTest.rb @@ -1 +1,2 @@ exclude :test_lock_sending_custom_lock_statement, 'AR looks for $1 when we use ?' if ActiveRecord::Base.lease_connection.prepared_statements +exclude :test_with_lock_locks_with_no_args, 'AR looks for $1 when we use ?' if ActiveRecord::Base.lease_connection.prepared_statements diff --git a/test/rails/excludes/postgresql/RelationMergingTest.rb b/test/rails/excludes/postgresql/RelationMergingTest.rb new file mode 100644 index 000000000..b4f3eb5ac --- /dev/null +++ b/test/rails/excludes/postgresql/RelationMergingTest.rb @@ -0,0 +1 @@ +exclude :test_merge_doesnt_duplicate_same_clauses, 'AR looks for $1 when we use ?' if ActiveRecord::Base.lease_connection.prepared_statements diff --git a/test/rails/excludes/postgresql/SanitizeTest.rb b/test/rails/excludes/postgresql/SanitizeTest.rb new file mode 100644 index 000000000..71fa2521b --- /dev/null +++ b/test/rails/excludes/postgresql/SanitizeTest.rb @@ -0,0 +1 @@ +exclude :test_sanitize_sql_like_example_use_case, 'AR looks for $1 when we use ?' if ActiveRecord::Base.lease_connection.prepared_statements diff --git a/test/rails/excludes/sqlite3/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb b/test/rails/excludes/sqlite3/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb new file mode 100644 index 000000000..ff8ea495d --- /dev/null +++ b/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" diff --git a/test/rails/excludes/sqlserver/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb b/test/rails/excludes/sqlserver/ActiveRecord/ConnectionAdapters/PoolConfig/ResolverTest.rb new file mode 100644 index 000000000..ff8ea495d --- /dev/null +++ b/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" diff --git a/test/simple.rb b/test/simple.rb index 2e6610d71..ddaa3b878 100644 --- a/test/simple.rb +++ b/test/simple.rb @@ -335,6 +335,22 @@ def test_time_with_default_timezone_local # + def test_preserving_time_objects_with_local_time_conversion_to_default_timezone_utc + skip "with_system_tz not working in tomcat" if ActiveRecord::Base.connection.raw_connection.jndi? + + with_system_tz 'America/New_York' do # with_env_tz in Rails' tests + with_timezone_config default: :utc do + time = Time.local(2000) + record = DbType.create!('sample_datetime' => time) + saved_time = record.class.find(record.id).reload.sample_datetime + + assert_equal time, saved_time + assert_equal [0, 0, 0, 1, 1, 2000, 6, 1, false, 'EST'], time.to_a + assert_equal [0, 0, 5, 1, 1, 2000, 6, 1, false, 'UTC'], saved_time.to_a + end + end + end + def test_preserving_time_objects_with_utc_time_conversion_to_default_timezone_local skip "with_system_tz not working in tomcat" if ActiveRecord::Base.connection.raw_connection.jndi?