Merge pull request rails#43105 from eileencodes/make-schema-cache-methods-consistent

Make schema cache methods behave consistently

Author: eileencodes

activerecord/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Make schema cache methods return consistent results.
+
+    Previously the schema cache methods `primary_keys`, `columns, `columns_hash`, and `indexes`
+    would behave differently than one another when a table didn't exist and differently across
+    databases. This change unifies the behavior so that all the methods return `nil` if the table
+    doesn't exist. While this class is technically public, applications don't interact with the
+    return values of these methods so it should be safe to make this change.
+
+    *Eileen M. Uchitelle*
+
 *   Reestablish connection to previous database after after running `db:schema:load:name`
 
     After running `db:schema:load:name` the previous connection is restored.

activerecord/lib/active_record/connection_adapters/schema_cache.rb

@@ -128,7 +128,11 @@ def columns_hash?(table_name)
 
       def indexes(table_name)
         @indexes.fetch(table_name) do
-          @indexes[deep_deduplicate(table_name)] = deep_deduplicate(connection.indexes(table_name))
+          if data_source_exists?(table_name)
+            @indexes[deep_deduplicate(table_name)] = deep_deduplicate(connection.indexes(table_name))
+          else
+            []
+          end
         end
       end
 

activerecord/test/cases/connection_adapters/schema_cache_test.rb

@@ -11,10 +11,6 @@ def setup
         @database_version = @connection.get_database_version
       end
 
-      def test_primary_key
-        assert_equal "id", @cache.primary_keys("posts")
-      end
-
       def test_yaml_dump_and_load
         # Create an empty cache.
         cache = SchemaCache.new @connection
@@ -119,23 +115,40 @@ def test_yaml_loads_5_1_dump_without_database_version_still_queries_for_database
         assert_equal @database_version.to_s, cache.database_version.to_s
       end
 
+      def test_primary_key_for_existent_table
+        assert_equal "id", @cache.primary_keys("posts")
+      end
+
       def test_primary_key_for_non_existent_table
         assert_nil @cache.primary_keys("omgponies")
       end
 
-      def test_caches_columns
-        columns = @cache.columns("posts")
-        assert_equal columns, @cache.columns("posts")
+      def test_columns_for_existent_table
+        assert_equal 12, @cache.columns("posts").size
+      end
+
+      def test_columns_for_non_existent_table
+        assert_raises ActiveRecord::StatementInvalid do
+          @cache.columns("omgponies")
+        end
+      end
+
+      def test_columns_hash_for_existent_table
+        assert_equal 12, @cache.columns_hash("posts").size
+      end
+
+      def test_columns_hash_for_non_existent_table
+        assert_raises ActiveRecord::StatementInvalid do
+          @cache.columns_hash("omgponies")
+        end
       end
 
-      def test_caches_columns_hash
-        columns_hash = @cache.columns_hash("posts")
-        assert_equal columns_hash, @cache.columns_hash("posts")
+      def test_indexes_for_existent_table
+        assert_equal 1, @cache.indexes("posts").size
       end
 
-      def test_caches_indexes
-        indexes = @cache.indexes("posts")
-        assert_equal indexes, @cache.indexes("posts")
+      def test_indexes_for_non_existent_table
+        assert_equal [], @cache.indexes("omgponies")
       end
 
       def test_caches_database_version