Commit bca193b0 authored by Yannis Roussos's avatar Yannis Roussos

Update database helpers to set the current_schema

- Update all migration and schema helpers that access
pg_class, pg_index, pg_constraing and similar catalog
yables to properly set the current schema when fetching
information about database objects.
- Update ReltuplesCountStrategy.get_statistics to use
the current schema so that we don’t accidentally fetch
the statistics for a table with the same name in a
schema different than the one GitLab uses.
- Add specs to check that migration helpers do not
generate conflicts in the presence of multiple schemas
with the same object names.
parent 075120fd
---
title: Update database helpers to set the current_schema
merge_request: 43568
author:
type: fixed
......@@ -74,8 +74,9 @@ module Gitlab
def get_statistics(table_names, check_statistics: true)
time = 6.hours.ago
query = PgClass.joins("LEFT JOIN pg_stat_user_tables USING (relname)")
query = PgClass.joins("LEFT JOIN pg_stat_user_tables ON pg_stat_user_tables.relid = pg_class.oid")
.where(relname: table_names)
.where('schemaname = current_schema()')
.select('pg_class.relname AS table_name, reltuples::bigint AS estimate')
if check_statistics
......
......@@ -994,10 +994,10 @@ into similar problems in the future (e.g. when new tables are created).
def postgres_exists_by_name?(table, name)
index_sql = <<~SQL
SELECT COUNT(*)
FROM pg_index
JOIN pg_class i ON (indexrelid=i.oid)
JOIN pg_class t ON (indrelid=t.oid)
WHERE i.relname = '#{name}' AND t.relname = '#{table}'
FROM pg_catalog.pg_indexes
WHERE schemaname = #{connection.quote(current_schema)}
AND tablename = #{connection.quote(table)}
AND indexname = #{connection.quote(name)}
SQL
connection.select_value(index_sql).to_i > 0
......@@ -1053,11 +1053,15 @@ into similar problems in the future (e.g. when new tables are created).
# the table name in addition to using the constraint_name
check_sql = <<~SQL
SELECT COUNT(*)
FROM pg_constraint
JOIN pg_class ON pg_constraint.conrelid = pg_class.oid
WHERE pg_constraint.contype = 'c'
AND pg_constraint.conname = '#{constraint_name}'
AND pg_class.relname = '#{table}'
FROM pg_catalog.pg_constraint con
INNER JOIN pg_catalog.pg_class rel
ON rel.oid = con.conrelid
INNER JOIN pg_catalog.pg_namespace nsp
ON nsp.oid = con.connamespace
WHERE con.contype = 'c'
AND con.conname = #{connection.quote(constraint_name)}
AND nsp.nspname = #{connection.quote(current_schema)}
AND rel.relname = #{connection.quote(table)}
SQL
connection.select_value(check_sql) > 0
......@@ -1284,8 +1288,9 @@ into similar problems in the future (e.g. when new tables are created).
check_sql = <<~SQL
SELECT c.is_nullable
FROM information_schema.columns c
WHERE c.table_name = '#{table}'
AND c.column_name = '#{column}'
WHERE c.table_schema = #{connection.quote(current_schema)}
AND c.table_name = #{connection.quote(table)}
AND c.column_name = #{connection.quote(column)}
SQL
connection.select_value(check_sql) == 'YES'
......
......@@ -32,11 +32,14 @@ module Gitlab
def trigger_exists?(table_name, name)
connection.select_value(<<~SQL)
SELECT 1
FROM pg_trigger
INNER JOIN pg_class
ON pg_trigger.tgrelid = pg_class.oid
WHERE pg_class.relname = '#{table_name}'
AND pg_trigger.tgname = '#{name}'
FROM pg_catalog.pg_trigger trgr
INNER JOIN pg_catalog.pg_class rel
ON trgr.tgrelid = rel.oid
INNER JOIN pg_catalog.pg_namespace nsp
ON nsp.oid = rel.relnamespace
WHERE nsp.nspname = #{connection.quote(current_schema)}
AND rel.relname = #{connection.quote(table_name)}
AND trgr.tgname = #{connection.quote(name)}
SQL
end
......
......@@ -1458,15 +1458,32 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
)
end
after do
'DROP INDEX IF EXISTS test_index;'
end
it 'returns true if an index exists' do
expect(model.index_exists_by_name?(:projects, 'test_index'))
.to be_truthy
end
end
context 'when an index exists for a table with the same name in another schema' do
before do
ActiveRecord::Base.connection.execute(
'CREATE SCHEMA new_test_schema'
)
ActiveRecord::Base.connection.execute(
'CREATE TABLE new_test_schema.projects (id integer, name character varying)'
)
ActiveRecord::Base.connection.execute(
'CREATE INDEX test_index_on_name ON new_test_schema.projects (LOWER(name));'
)
end
it 'returns false if the index does not exist in the current schema' do
expect(model.index_exists_by_name?(:projects, 'test_index_on_name'))
.to be_falsy
end
end
end
describe '#create_or_update_plan_limit' do
......@@ -1921,11 +1938,17 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
ActiveRecord::Base.connection.execute(
'ALTER TABLE projects ADD CONSTRAINT check_1 CHECK (char_length(path) <= 5) NOT VALID'
)
end
after do
ActiveRecord::Base.connection.execute(
'ALTER TABLE projects DROP CONSTRAINT IF EXISTS check_1'
'CREATE SCHEMA new_test_schema'
)
ActiveRecord::Base.connection.execute(
'CREATE TABLE new_test_schema.projects (id integer, name character varying)'
)
ActiveRecord::Base.connection.execute(
'ALTER TABLE new_test_schema.projects ADD CONSTRAINT check_2 CHECK (char_length(name) <= 5)'
)
end
......@@ -1943,6 +1966,11 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
expect(model.check_constraint_exists?(:users, 'check_1'))
.to be_falsy
end
it 'returns false if a constraint with the same name exists for the same table in another schema' do
expect(model.check_constraint_exists?(:projects, 'check_2'))
.to be_falsy
end
end
describe '#add_check_constraint' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment