Merge pull request #997 from dr-itz/50-dev

50.x fixes

Author: enebo

.travis.yml

@@ -42,6 +42,8 @@ rvm:
   - jruby-9.1.16.0
 jdk:
   - openjdk8
+addons:
+  postgresql: "9.4"
 env:
   - DB=mysql2 PREPARED_STATEMENTS=false
   - DB=mysql2 PREPARED_STATEMENTS=true

lib/arjdbc/abstract/database_statements.rb

@@ -26,8 +26,8 @@ def exec_query(sql, name = nil, binds = NO_BINDS, prepare: false)
           log(sql, name) { @connection.execute_query(sql) }
         else
           log(sql, name, binds) do
-            # It seems that #supports_statement_cache? is defined but isn't checked before setting "prepare" (AR 5.0)
-            cached_statement = fetch_cached_statement(sql) if prepare && supports_statement_cache?
+            # this is different from normal AR that always caches
+            cached_statement = fetch_cached_statement(sql) if prepare && @jdbc_statement_cache_enabled
             @connection.execute_prepared_query(sql, binds, cached_statement)
           end
         end

lib/arjdbc/abstract/statement_cache.rb

@@ -23,7 +23,7 @@ def initialize(*args) # (connection, logger, config)
         # Only say we support the statement cache if we are using prepared statements
         # and have a max number of statements defined
         statement_limit = self.class.type_cast_config_to_integer(config[:statement_limit])
-        @jdbc_statement_cache_enabled = config[:prepared_statements] && (statement_limit.nil? || statement_limit > 0)
+        @jdbc_statement_cache_enabled = prepared_statements && (statement_limit.nil? || statement_limit > 0)
 
         @statements = StatementPool.new(statement_limit) # AR (5.0) expects this to be stored as @statements
       end
@@ -34,11 +34,11 @@ def clear_cache!
       end
 
       def delete_cached_statement(sql)
-        @statements.delete(cached_statement_key(sql))
+        @statements.delete(sql_key(sql))
       end
 
       def fetch_cached_statement(sql)
-        @statements[cached_statement_key(sql)] ||= @connection.connection.prepare_statement(sql)
+        @statements[sql_key(sql)] ||= @connection.prepare_statement(sql)
       end
 
       def supports_statement_cache?
@@ -49,7 +49,7 @@ def supports_statement_cache?
 
       # This should be overridden by the adapter if the sql itself
       # is not enough to make the key unique
-      def cached_statement_key(sql)
+      def sql_key(sql)
         sql
       end
 

lib/arjdbc/postgresql/_bc_time_cast_patch.rb

@@ -522,61 +522,6 @@ def truncate(table_name, name = nil)
       execute "TRUNCATE TABLE #{quote_table_name(table_name)}", name
     end
 
-    # Returns an array of indexes for the given table.
-    def indexes(table_name, name = nil)
-      # FIXME: AR version => table = Utils.extract_schema_qualified_name(table_name.to_s)
-      schema, table = extract_schema_and_table(table_name.to_s)
-
-      result = query(<<-SQL, 'SCHEMA')
-            SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid,
-                            pg_catalog.obj_description(i.oid, 'pg_class') AS comment,
-            (SELECT COUNT(*) FROM pg_opclass o
-               JOIN (SELECT unnest(string_to_array(d.indclass::text, ' '))::int oid) c
-                 ON o.oid = c.oid WHERE o.opcdefault = 'f')
-            FROM pg_class t
-            INNER JOIN pg_index d ON t.oid = d.indrelid
-            INNER JOIN pg_class i ON d.indexrelid = i.oid
-            LEFT JOIN pg_namespace n ON n.oid = i.relnamespace
-            WHERE i.relkind = 'i'
-              AND d.indisprimary = 'f'
-              AND t.relname = '#{table}'
-              AND n.nspname = #{schema ? "'#{schema}'" : 'ANY (current_schemas(false))'}
-            ORDER BY i.relname
-      SQL
-
-      result.map do |row|
-        index_name = row[0]
-        # FIXME: These values [1,2] are returned in a different format than AR expects, maybe we could update it on the Java side to be more accurate
-        unique = row[1].is_a?(String) ? row[1] == 't' : row[1] # JDBC gets us a boolean
-        indkey = row[2].is_a?(Java::OrgPostgresqlUtil::PGobject) ? row[2].value : row[2]
-        indkey = indkey.split(" ").map(&:to_i)
-        inddef = row[3]
-        oid = row[4]
-        comment = row[5]
-        opclass = row[6]
-
-        using, expressions, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: WHERE (.+))?\z/).flatten
-
-        if indkey.include?(0) || opclass > 0
-          columns = expressions
-        else
-          columns = Hash[query(<<-SQL.strip_heredoc, "SCHEMA")].values_at(*indkey).compact
-                SELECT a.attnum, a.attname
-                FROM pg_attribute a
-                WHERE a.attrelid = #{oid}
-                AND a.attnum IN (#{indkey.join(",")})
-          SQL
-
-          # add info on sort order for columns (only desc order is explicitly specified, asc is the default)
-          orders = Hash[
-              expressions.scan(/(\w+) DESC/).flatten.map { |order_column| [order_column, :desc] }
-          ]
-        end
-
-        IndexDefinition.new(table_name, index_name, unique, columns, [], orders, where, nil, using.to_sym, comment.presence)
-      end.compact
-    end
-
     # @private
     def column_name_for_operation(operation, node)
       case operation
@@ -682,8 +627,6 @@ class PostgreSQLAdapter < AbstractAdapter
     require 'arjdbc/postgresql/oid_types'
     include ::ArJdbc::PostgreSQL::OIDTypes
 
-    load 'arjdbc/postgresql/_bc_time_cast_patch.rb'
-
     include ::ArJdbc::PostgreSQL::ColumnHelpers
 
     include ::ArJdbc::Util::QuotedCache
@@ -736,7 +679,7 @@ def jdbc_connection_class(spec)
 
     # Prepared statements aren't schema aware so we need to make sure we
     # store different PreparedStatement objects for different schemas
-    def cached_statement_key(sql)
+    def sql_key(sql)
       "#{schema_search_path}-#{sql}"
     end
 

lib/arjdbc/postgresql/adapter.rb

@@ -1112,6 +1112,22 @@ public IRubyObject call(final Connection connection) throws SQLException {
         });
     }
 
+    /**
+     * Prepares a query, returns a wrapped PreparedStatement. This takes care of exception wrapping
+     * @param context which context this method is executing on.
+     * @param sql the query to prepare-
+     * @return a Ruby <code>PreparedStatement</code>
+     */
+    @JRubyMethod(required = 1)
+    public IRubyObject prepare_statement(final ThreadContext context, final IRubyObject sql) {
+        return withConnection(context, new Callable<IRubyObject>() {
+            public IRubyObject call(Connection connection) throws SQLException {
+                final String query = sql.convertToString().getUnicodeValue();
+                return JavaUtil.convertJavaToRuby(context.runtime, connection.prepareStatement(query));
+            }
+        });
+    }
+
     // Called from exec_query in abstract/database_statements
     /**
      * Executes a query and returns the (AR) result.  There are three parameters:

src/java/arjdbc/jdbc/RubyJdbcConnection.java

@@ -292,17 +292,6 @@ def test_bc_timestamp
     time = Time.utc('0000-01-01') - 1.hour - 1.minute - 1.second
     db_type = DbType.create!(:sample_timestamp => time)
 
-    if current_connection_config[:prepared_statements].to_s == 'true'
-      pend "Likely a JRuby/Java thing - this test is failing bad: check #516"
-    end
-    # if current_connection_config[:insert_returning].to_s == 'true'
-    #   pend "BC timestamps not-handled right with INSERT RETURNIG ..."
-    # end unless ar_version('4.2')
-    #
-    if defined?(JRUBY_VERSION) && JRUBY_VERSION < '9.2'
-      pend "BC timestamp handling isn't working properly through JRuby 9.1 (its to be fixed in 9.2)"
-    end
-
     assert_equal time, db_type.reload.sample_timestamp
 
     date = DateTime.parse('0000-01-01T00:00:00+00:00') - 1.hour - 1.minute - 1.second

test/db/postgresql/simple_test.rb

@@ -1,2 +1,3 @@
 exclude :test_connection_options, 'Connection options not supported'
 exclude :test_table_alias_length_logs_name, 'We reuse max identifer length for this so no query is made to be logged for the rails test'
+exclude :test_statement_key_is_logged, 'test uses $1 instead of ? for bind'

test/rails/excludes/postgresql/ActiveRecord/PostgresqlConnectionTest.rb

@@ -0,0 +1 @@
+exclude :test_auth_with_bind, 'test uses $1 instead of ? for bind'