Skip to content

Commit e323e68

Browse files
committed
Make schema cache methods behave consistently
In rails#43075 I added the ability to skip adding a configured set of tables to the schema cache. After pushing that PR I found the behavior across methods and database adapters to be inconsistent when a table didn't exist. Since ignored tables should behave the same as missing tables, I wanted to make the behavior consistent. Previously the behavior was: *mysql* columns & columns_hash: Raised db error for non-existent table primary_keys: returned `nil` for non-existent table indexes: Raised db error for non-existent table *sqlite3* columns & columns_hash: Raised db error for non-existent table primary_keys: returned `nil` for non-existent table indexes: returned `[]` for non-existent table *postgresql* columns & columns_hash: Raised db error for non-existent table primary_keys: returned `nil` for non-existent table indexes: returned `[]` for non-existent table Current behavior: *mysql, sqlite3, and postgresql* columns & columns_hash: Raise db error for non-existent table primary_keys: returns `nil` for non-existent table indexes: returns `[]` for non-existent table The only change is to mysql, which used to raise for indexes, now it returns and empty array like all the other adapters.
1 parent c70e264 commit e323e68

File tree

3 files changed

+41
-14
lines changed

3 files changed

+41
-14
lines changed

activerecord/CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
* Make schema cache methods return consistent results.
2+
3+
Previously the schema cache methods `primary_keys`, `columns, `columns_hash`, and `indexes`
4+
would behave differently than one another when a table didn't exist and differently across
5+
databases. This change unifies the behavior so that all the methods return `nil` if the table
6+
doesn't exist. While this class is technically public, applications don't interact with the
7+
return values of these methods so it should be safe to make this change.
8+
9+
*Eileen M. Uchitelle*
10+
111
* Reestablish connection to previous database after after running `db:schema:load:name`
212

313
After running `db:schema:load:name` the previous connection is restored.

activerecord/lib/active_record/connection_adapters/schema_cache.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,11 @@ def columns_hash?(table_name)
128128

129129
def indexes(table_name)
130130
@indexes.fetch(table_name) do
131-
@indexes[deep_deduplicate(table_name)] = deep_deduplicate(connection.indexes(table_name))
131+
if data_source_exists?(table_name)
132+
@indexes[deep_deduplicate(table_name)] = deep_deduplicate(connection.indexes(table_name))
133+
else
134+
[]
135+
end
132136
end
133137
end
134138

activerecord/test/cases/connection_adapters/schema_cache_test.rb

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ def setup
1111
@database_version = @connection.get_database_version
1212
end
1313

14-
def test_primary_key
15-
assert_equal "id", @cache.primary_keys("posts")
16-
end
17-
1814
def test_yaml_dump_and_load
1915
# Create an empty cache.
2016
cache = SchemaCache.new @connection
@@ -119,23 +115,40 @@ def test_yaml_loads_5_1_dump_without_database_version_still_queries_for_database
119115
assert_equal @database_version.to_s, cache.database_version.to_s
120116
end
121117

118+
def test_primary_key_for_existent_table
119+
assert_equal "id", @cache.primary_keys("posts")
120+
end
121+
122122
def test_primary_key_for_non_existent_table
123123
assert_nil @cache.primary_keys("omgponies")
124124
end
125125

126-
def test_caches_columns
127-
columns = @cache.columns("posts")
128-
assert_equal columns, @cache.columns("posts")
126+
def test_columns_for_existent_table
127+
assert_equal 12, @cache.columns("posts").size
128+
end
129+
130+
def test_columns_for_non_existent_table
131+
assert_raises ActiveRecord::StatementInvalid do
132+
@cache.columns("omgponies")
133+
end
134+
end
135+
136+
def test_columns_hash_for_existent_table
137+
assert_equal 12, @cache.columns_hash("posts").size
138+
end
139+
140+
def test_columns_hash_for_non_existent_table
141+
assert_raises ActiveRecord::StatementInvalid do
142+
@cache.columns_hash("omgponies")
143+
end
129144
end
130145

131-
def test_caches_columns_hash
132-
columns_hash = @cache.columns_hash("posts")
133-
assert_equal columns_hash, @cache.columns_hash("posts")
146+
def test_indexes_for_existent_table
147+
assert_equal 1, @cache.indexes("posts").size
134148
end
135149

136-
def test_caches_indexes
137-
indexes = @cache.indexes("posts")
138-
assert_equal indexes, @cache.indexes("posts")
150+
def test_indexes_for_non_existent_table
151+
assert_equal [], @cache.indexes("omgponies")
139152
end
140153

141154
def test_caches_database_version

0 commit comments

Comments
 (0)