From cfc74f19fa526f3a68095595855b631edcf70b39 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 28 Jun 2025 14:08:26 -0700 Subject: [PATCH] Add support for sequences for database validation schema This adds basic support for identifying missing database sequences. This does not yet: * Check ownership of sequences * Check that columns have properly been assigned to the right sequence. Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/396878 --- .../lib/gitlab/schema/validation.rb | 5 + .../adapters/sequence_database_adapter.rb | 59 +++++ .../sequence_structure_sql_adapter.rb | 62 +++++ .../validation/schema_objects/sequence.rb | 39 +++ .../schema/validation/sources/database.rb | 56 ++++ .../sources/sequence_structure_sql_parser.rb | 185 +++++++++++++ .../validation/sources/structure_sql.rb | 22 +- .../schema/validation/validators/base.rb | 1 + .../validators/missing_sequences.rb | 21 ++ .../spec/fixtures/structure.sql | 4 + .../sequence_database_adapter_spec.rb | 170 ++++++++++++ .../sequence_structure_sql_adapter_spec.rb | 160 ++++++++++++ .../sequence_structure_sql_parser_spec.rb | 247 ++++++++++++++++++ .../schema/validation/validators/base_spec.rb | 1 + .../validators/missing_sequences_spec.rb | 13 + .../sequence_validators_shared_examples.rb | 39 +++ 16 files changed, 1083 insertions(+), 1 deletion(-) create mode 100644 gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_database_adapter.rb create mode 100644 gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter.rb create mode 100644 gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb create mode 100644 gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser.rb create mode 100644 gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/missing_sequences.rb create mode 100644 gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_database_adapter_spec.rb create mode 100644 gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter_spec.rb create mode 100644 gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser_spec.rb create mode 100644 gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb create mode 100644 gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb index 2993ef814f43d3..d1cfeddee123b7 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb @@ -15,6 +15,7 @@ require_relative 'validation/validators/different_definition_indexes' require_relative 'validation/validators/extra_indexes' require_relative 'validation/validators/missing_indexes' +require_relative 'validation/validators/missing_sequences' require_relative 'validation/validators/extra_table_columns' require_relative 'validation/validators/missing_table_columns' @@ -35,12 +36,14 @@ require_relative 'validation/sources/connection_adapters/active_record_adapter' require_relative 'validation/sources/connection_adapters/pg_adapter' require_relative 'validation/sources/structure_sql' +require_relative 'validation/sources/sequence_structure_sql_parser' require_relative 'validation/sources/database' require_relative 'validation/sources/connection' require_relative 'validation/schema_objects/base' require_relative 'validation/schema_objects/column' require_relative 'validation/schema_objects/index' +require_relative 'validation/schema_objects/sequence' require_relative 'validation/schema_objects/table' require_relative 'validation/schema_objects/trigger' require_relative 'validation/schema_objects/foreign_key' @@ -49,6 +52,8 @@ require_relative 'validation/adapters/column_structure_sql_adapter' require_relative 'validation/adapters/foreign_key_database_adapter' require_relative 'validation/adapters/foreign_key_structure_sql_adapter' +require_relative 'validation/adapters/sequence_database_adapter' +require_relative 'validation/adapters/sequence_structure_sql_adapter' module Gitlab module Schema diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_database_adapter.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_database_adapter.rb new file mode 100644 index 00000000000000..e8eaaf9a1217da --- /dev/null +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_database_adapter.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Gitlab + module Schema + module Validation + module Adapters + class SequenceDatabaseAdapter + def initialize(query_result) + @query_result = query_result + end + + def name + return unless query_result['sequence_name'] + + "#{schema}.#{query_result['sequence_name']}" + end + + def column_owner + return unless query_result['owned_by_column'] + + "#{schema}.#{query_result['owned_by_column']}" + end + + def user_owner + query_result['user_owner'] + end + + def start_value + query_result['start_value'] + end + + def increment_by + query_result['increment_by'] + end + + def min_value + query_result['min_value'] + end + + def max_value + query_result['max_value'] + end + + def cycle + query_result['cycle'] + end + + private + + attr_reader :query_result + + def schema + query_result['schema'] || 'public' + end + end + end + end + end +end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter.rb new file mode 100644 index 00000000000000..9aa1becd5eef8f --- /dev/null +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Gitlab + module Schema + module Validation + module Adapters + class SequenceStructureSqlAdapter + attr_reader :sequence_name, :schema_name + attr_accessor :owner_table, :owner_column, :owner_schema + + def initialize( + sequence_name:, schema_name: nil, owner_table: nil, + owner_column: nil, owner_schema: nil) + @sequence_name = sequence_name + @schema_name = schema_name + @owner_table = owner_table + @owner_column = owner_column + @owner_schema = owner_schema + end + + # Fully qualified sequence name (schema.sequence_name) + def name + "#{schema}.#{sequence_name}" + end + + # Just the column name + def column_name + owner_column + end + + def table_name + owner_table + end + + # Fully qualified column reference (schema.table.column) + def column_owner + return unless owner_table && owner_column + + "#{column_schema}.#{owner_table}.#{owner_column}" + end + + # Get the schema this sequence belongs to + def schema + schema_name || owner_schema || 'public' + end + + def column_schema + owner_schema || schema_name || 'public' + end + + def to_s + "SequenceStructureSqlAdapter(#{name} -> #{column_owner})" + end + + def inspect + "# #{column_owner}>" + end + end + end + end + end +end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb new file mode 100644 index 00000000000000..4c00d5f3aa5fcf --- /dev/null +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module Schema + module Validation + module SchemaObjects + class Sequence < Base + def initialize(adapter) + @adapter = adapter + end + + # Sequence should include the schema, as the same name could be used across different schemas + # + # @example public.sequence_name + def name + @name ||= adapter.name + end + + # Fully qualified column reference (schema.table.column) + def owner + @owner ||= adapter.column_owner + end + + def table_name + @table_name ||= adapter.table_name + end + + def statement + "CREATE SEQUENCE #{name}" + end + + private + + attr_reader :adapter + end + end + end + end +end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/database.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/database.rb index 3f62396c156e43..6a1c28e1fc6383 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/database.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/database.rb @@ -27,6 +27,10 @@ def fetch_table_by_name(table_name) table_map[table_name] end + def fetch_sequence_by_name(sequence_name) + sequence_map[sequence_name] + end + def index_exists?(index_name) index = index_map[index_name] @@ -59,6 +63,10 @@ def table_exists?(table_name) true end + def sequence_exists?(sequence_name) + !!fetch_sequence_by_name(sequence_name) + end + def indexes index_map.values end @@ -75,6 +83,10 @@ def tables table_map.values end + def sequences + sequence_map.values + end + private attr_reader :connection @@ -113,6 +125,12 @@ def table_map end end + def sequence_map + @sequence_map ||= fetch_sequences.transform_values! do |stmt| + SchemaObjects::Sequence.new(Adapters::SequenceDatabaseAdapter.new(stmt.first)) + end + end + def fetch_tables # rubocop:disable Rails/SquishedSQLHeredocs sql = <<~SQL @@ -187,6 +205,44 @@ def fetch_fks connection.exec_query(sql, [connection.current_schema]) end + + # Fetch all the sequences + def fetch_sequences + # rubocop:disable Rails/SquishedSQLHeredocs + sql = <<~SQL + SELECT + c.relname AS sequence_name, + n.nspname AS schema, + pg_catalog.pg_get_userbyid(c.relowner) AS user_owner, + s.seqstart AS start_value, + s.seqincrement AS increment_by, + s.seqmin AS min_value, + s.seqmax AS max_value, + s.seqcycle AS cycle, + s.seqcache AS cache_size, + pg_catalog.obj_description(c.oid, 'pg_class') AS comment, + CASE + WHEN d.refobjid IS NOT NULL THEN + ref_class.relname || '.' || ref_attr.attname + ELSE NULL + END AS owned_by_column + FROM pg_catalog.pg_class c + INNER JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + LEFT JOIN pg_catalog.pg_sequence s ON s.seqrelid = c.oid + LEFT JOIN pg_catalog.pg_depend d ON d.objid = c.oid + AND d.deptype = 'a' + AND d.classid = 'pg_class'::regclass + LEFT JOIN pg_catalog.pg_class ref_class ON ref_class.oid = d.refobjid + LEFT JOIN pg_catalog.pg_attribute ref_attr ON ref_attr.attrelid = d.refobjid + AND ref_attr.attnum = d.refobjsubid + WHERE c.relkind = 'S' + AND n.nspname IN ($1, $2) + ORDER BY c.relname, n.nspname + SQL + # rubocop:enable Rails/SquishedSQLHeredocs + + connection.exec_query(sql, schemas).group_by { |seq| "#{seq['schema']}.#{seq['sequence_name']}" } + end end end end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser.rb new file mode 100644 index 00000000000000..429c331aae5fae --- /dev/null +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +module Gitlab + module Schema + module Validation + module Sources + class SequenceStructureSqlParser + attr_reader :sequences + + def initialize(parsed_structure, default_schema_name) + @parsed_structure = parsed_structure + @default_schema_name = default_schema_name + @sequences = {} + end + + # Returns a map of sequence name to sequence structure objects + def execute + extract_sequences + @sequences + end + + private + + attr_reader :parsed_structure, :default_schema_name + + def extract_sequences + parsed_structure.tree.stmts.each do |stmt| + case stmt.stmt.node + when :create_seq_stmt + process_create_sequence(stmt.stmt.create_seq_stmt) + when :alter_seq_stmt + process_alter_sequence(stmt.stmt.alter_seq_stmt) + when :alter_table_stmt + process_alter_table(stmt.stmt.alter_table_stmt) + end + end + end + + # Process CREATE SEQUENCE SQL queries. For example: + # + # CREATE SEQUENCE web_hook_logs_id_seq + def process_create_sequence(create_seq) + sequence_name = create_seq.sequence.relname + schema_name = resolve_schema_name(create_seq.sequence.schemaname) + full_name = "#{schema_name}.#{sequence_name}" + + @sequences[full_name] = ::Gitlab::Schema::Validation::Adapters::SequenceStructureSqlAdapter.new( + sequence_name: sequence_name, + schema_name: schema_name + ) + end + + # Processes ALTER SEQUENCE SQL queries to extract column owner. For example: + # + # ALTER SEQUENCE ai_code_suggestion_events_id_seq OWNED BY ai_code_suggestion_events.id; + def process_alter_sequence(alter_seq) + sequence_schema = alter_seq.sequence.schemaname + sequence_schema = default_schema_name if sequence_schema == '' + sequence_name = alter_seq.sequence.relname + + # Look for OWNED BY option + return unless alter_seq.options + + owner_schema = default_schema_name + owner_table = nil + owner_column = nil + + alter_seq.options.each do |option| + def_elem = option.def_elem + + next unless def_elem.defname == 'owned_by' + next unless def_elem.arg && def_elem.arg.node == :list + + owned_by_list = def_elem.arg.list.items + + next unless owned_by_list.length >= 2 + + # Handle schema.table.column or table.column + if owned_by_list.length == 3 + owner_schema = owned_by_list[0].string.sval + owner_table = owned_by_list[1].string.sval + owner_column = owned_by_list[2].string.sval + else + owner_table = owned_by_list[0].string.sval + owner_column = owned_by_list[1].string.sval + end + end + + full_name = "#{sequence_schema}.#{sequence_name}" + # Update or create sequence with ownership info + existing = @sequences[full_name] + + unless existing + warn "Could not find sequence #{full_name} for ALTER SEQUENCE command" + return + end + + existing.owner_table = owner_table + existing.owner_column = owner_column + existing.owner_schema = owner_schema + end + + # Process ALTER TABLE commands to extract sequence owner. For example: + # + # ALTER TABLE ONLY web_hook_logs ALTER COLUMN id SET DEFAULT nextval('web_hook_logs_id_seq'::regclass); + def process_alter_table(alter_table) + table_name = alter_table.relation.relname + table_schema = resolve_schema_name(alter_table.relation.schemaname) + + alter_table.cmds.each do |cmd| + alter_cmd = cmd.alter_table_cmd + + # Look for SET DEFAULT nextval(...) commands + next unless alter_cmd.subtype == :AT_ColumnDefault + + column_name = alter_cmd.name + sequence_name = extract_sequence_from_default(alter_cmd.def) + sequence_schema, sequence_name = process_sequence_name(sequence_name) + lookup_name = "#{sequence_schema}.#{sequence_name}" + + # Update existing sequence or create new one + existing = @sequences[lookup_name] + + @sequences[lookup_name] = ::Gitlab::Schema::Validation::Adapters::SequenceStructureSqlAdapter.new( + sequence_name: sequence_name, + schema_name: existing&.schema_name || table_schema, + owner_table: existing&.owner_table || table_name, + owner_column: existing&.owner_column || column_name, + owner_schema: existing&.owner_schema || table_schema + ) + end + end + + def extract_sequence_from_default(expr) + return nil unless expr + + case expr.node + when :func_call + func_call = expr.func_call + return extract_string_from_expr(func_call.args.first) if nextval_func?(func_call) + when :type_cast + return extract_sequence_from_default(expr.type_cast.arg) + end + nil + end + + def extract_string_from_expr(expr) + case expr.node + when :a_const + expr.a_const.sval.sval if expr.a_const.val == :sval + when :type_cast + extract_string_from_expr(expr.type_cast.arg) + end + end + + def resolve_schema_name(schema_name) + return default_schema_name if schema_name == '' + + schema_name + end + + def process_sequence_name(sequence_name) + data = sequence_name.split('.', 2) + + return default_schema_name, sequence_name if data.length == 1 + + [data.first, data.last] + end + + def nextval_func?(func_call) + return false unless func_call.args.any? + + func_call.funcname.any? do |name| + sval = name.string&.sval + + next false unless sval + + sval.casecmp('nextval').zero? + end + end + end + end + end + end +end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/structure_sql.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/structure_sql.rb index aa7bfc7e43ef96..9a61369c1dab73 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/structure_sql.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/sources/structure_sql.rb @@ -32,6 +32,10 @@ def fetch_table_by_name(table_name) table_map[table_name] end + def fetch_sequence_by_name(sequence_name) + sequence_map[sequence_name] + end + def index_exists?(index_name) !!fetch_index_by_name(index_name) end @@ -48,6 +52,10 @@ def table_exists?(table_name) !!fetch_table_by_name(table_name) end + def sequence_exists?(sequence_name) + !!fetch_sequence_by_name(sequence_name) + end + def indexes @indexes ||= map_with_default_schema(index_statements, SchemaObjects::Index) end @@ -56,6 +64,18 @@ def triggers @triggers ||= map_with_default_schema(trigger_statements, SchemaObjects::Trigger) end + def sequences + sequence_map.values + end + + def sequence_map + @sequences ||= begin + parser = Gitlab::Schema::Validation::Sources::SequenceStructureSqlParser.new( + parsed_structure_file, schema_name) + parser.execute.transform_values! { |sequence| SchemaObjects::Sequence.new(sequence) } + end + end + def foreign_keys @foreign_keys ||= foreign_key_statements.map do |stmt| stmt.relation.schemaname = schema_name if stmt.relation.schemaname == '' @@ -132,7 +152,7 @@ def statements end def parsed_structure_file - PgQuery.parse(File.read(structure_file_path)) + @parsed_structure_file ||= PgQuery.parse(File.read(structure_file_path)) end def map_with_default_schema(statements, validation_class) diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb index 915bf70a7de55d..e3d02f869438a8 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb @@ -19,6 +19,7 @@ def self.all_validators MissingIndexes, MissingTriggers, MissingForeignKeys, + MissingSequences, DifferentDefinitionTables, DifferentDefinitionIndexes, DifferentDefinitionTriggers, diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/missing_sequences.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/missing_sequences.rb new file mode 100644 index 00000000000000..cf0958ef699f04 --- /dev/null +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/missing_sequences.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Schema + module Validation + module Validators + class MissingSequences < Base + ERROR_MESSAGE = "The sequence %s is missing from the database" + + def execute + structure_sql.sequences.filter_map do |sequence| + next if database.sequence_exists?(sequence.name) + + build_inconsistency(self.class, sequence, nil) + end + end + end + end + end + end +end diff --git a/gems/gitlab-schema-validation/spec/fixtures/structure.sql b/gems/gitlab-schema-validation/spec/fixtures/structure.sql index d8b6722d182e4d..7444324ac7b3f8 100644 --- a/gems/gitlab-schema-validation/spec/fixtures/structure.sql +++ b/gems/gitlab-schema-validation/spec/fixtures/structure.sql @@ -1,3 +1,7 @@ +CREATE SEQUENCE missing_sequence; +CREATE SEQUENCE shared_audit_event_id_seq; +CREATE SEQUENCE abuse_events_id_seq; + CREATE INDEX missing_index ON events USING btree (created_at, author_id); CREATE UNIQUE INDEX wrong_index ON table_name (column_name, column_name_2); diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_database_adapter_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_database_adapter_spec.rb new file mode 100644 index 00000000000000..ec46a581b7df5c --- /dev/null +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_database_adapter_spec.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Schema::Validation::Adapters::SequenceDatabaseAdapter do + let(:query_result) do + { + 'schema' => 'public', + 'sequence_name' => 'users_id_seq', + 'owned_by_column' => 'users.id', + 'user_owner' => 'gitlab', + 'start_value' => '1', + 'increment_by' => '1', + 'min_value' => '1', + 'max_value' => '9223372036854775807', + 'cycle' => false + } + end + + subject(:adapter) { described_class.new(query_result) } + + describe '#name' do + it 'returns formatted sequence name with schema' do + expect(adapter.name).to eq('public.users_id_seq') + end + + context 'when schema or sequence_name is nil' do + let(:query_result) { { 'schema' => nil, 'sequence_name' => 'test_seq' } } + + it 'defaults to public schema' do + expect(adapter.name).to eq('public.test_seq') + end + end + + context 'when both schema and sequence_name are missing' do + let(:query_result) { {} } + + it 'returns nil' do + expect(adapter.name).to be_nil + end + end + end + + describe '#column_owner' do + it 'returns formatted column owner with schema' do + expect(adapter.column_owner).to eq('public.users.id') + end + + context 'when owned_by_column contains table.column format' do + let(:query_result) do + { + 'schema' => 'analytics', + 'owned_by_column' => 'events.event_id' + } + end + + it 'prepends schema correctly' do + expect(adapter.column_owner).to eq('analytics.events.event_id') + end + end + + context 'when schema or owned_by_column is nil' do + let(:query_result) { { 'schema' => 'test', 'owned_by_column' => nil } } + + it 'returns nil' do + expect(adapter.column_owner).to be_nil + end + end + end + + describe '#user_owner' do + it 'returns the user owner' do + expect(adapter.user_owner).to eq('gitlab') + end + + context 'when user_owner is nil' do + let(:query_result) { { 'user_owner' => nil } } + + it 'returns nil' do + expect(adapter.user_owner).to be_nil + end + end + + context 'when user_owner is missing' do + let(:query_result) { {} } + + it 'returns nil' do + expect(adapter.user_owner).to be_nil + end + end + end + + describe '#start_value' do + it 'returns the start value' do + expect(adapter.start_value).to eq('1') + end + + context 'when start_value is different' do + let(:query_result) { { 'start_value' => '100' } } + + it 'returns the correct value' do + expect(adapter.start_value).to eq('100') + end + end + end + + describe '#increment_by' do + it 'returns the increment value' do + expect(adapter.increment_by).to eq('1') + end + + context 'when increment_by is different' do + let(:query_result) { { 'increment_by' => '5' } } + + it 'returns the correct value' do + expect(adapter.increment_by).to eq('5') + end + end + end + + describe '#min_value' do + it 'returns the minimum value' do + expect(adapter.min_value).to eq('1') + end + + context 'when min_value is different' do + let(:query_result) { { 'min_value' => '0' } } + + it 'returns the correct value' do + expect(adapter.min_value).to eq('0') + end + end + end + + describe '#max_value' do + it 'returns the maximum value' do + expect(adapter.max_value).to eq('9223372036854775807') + end + + context 'when max_value is different' do + let(:query_result) { { 'max_value' => '1000' } } + + it 'returns the correct value' do + expect(adapter.max_value).to eq('1000') + end + end + end + + describe '#cycle' do + it 'returns the cycle value' do + expect(adapter.cycle).to be(false) + end + + context 'when cycle is true' do + let(:query_result) { { 'cycle' => true } } + + it 'returns true' do + expect(adapter.cycle).to be(true) + end + end + + context 'when cycle is nil' do + let(:query_result) { { 'cycle' => nil } } + + it 'returns nil' do + expect(adapter.cycle).to be_nil + end + end + end +end diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter_spec.rb new file mode 100644 index 00000000000000..75d5f475fb13ec --- /dev/null +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/adapters/sequence_structure_sql_adapter_spec.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Schema::Validation::Adapters::SequenceStructureSqlAdapter do + let(:sequence_name) { 'users_id_seq' } + let(:schema_name) { 'public' } + let(:owner_table) { 'users' } + let(:owner_column) { 'id' } + let(:owner_schema) { 'app_schema' } + + subject(:sequence_sql_adapter) do + described_class.new( + sequence_name: sequence_name, + schema_name: schema_name, + owner_table: owner_table, + owner_column: owner_column, + owner_schema: owner_schema + ) + end + + describe '#initialize' do + context 'with all parameters' do + it 'sets all attributes correctly' do + expect(sequence_sql_adapter.sequence_name).to eq(sequence_name) + expect(sequence_sql_adapter.schema_name).to eq(schema_name) + expect(sequence_sql_adapter.owner_table).to eq(owner_table) + expect(sequence_sql_adapter.owner_column).to eq(owner_column) + expect(sequence_sql_adapter.owner_schema).to eq(owner_schema) + end + end + + context 'with minimal parameters' do + let(:schema_name) { nil } + let(:owner_table) { nil } + let(:owner_column) { nil } + let(:owner_schema) { nil } + + it 'sets sequence_name and defaults others to nil' do + expect(sequence_sql_adapter.sequence_name).to eq(sequence_name) + expect(sequence_sql_adapter.schema_name).to be_nil + expect(sequence_sql_adapter.owner_table).to be_nil + expect(sequence_sql_adapter.owner_column).to be_nil + expect(sequence_sql_adapter.owner_schema).to be_nil + end + end + end + + describe '#name' do + context 'when schema_name is present' do + let(:owner_table) { nil } + let(:owner_column) { nil } + let(:owner_schema) { nil } + + it 'returns fully qualified sequence name' do + expect(sequence_sql_adapter.name).to eq('public.users_id_seq') + end + end + + context 'when schema_name and owner_schema are nil' do + let(:schema_name) { nil } + let(:owner_schema) { nil } + + it 'returns sequence name with public schema' do + expect(sequence_sql_adapter.name).to eq('public.users_id_seq') + end + end + end + + describe '#column_name' do + it 'returns the owner_column' do + expect(sequence_sql_adapter.column_name).to eq(owner_column) + end + end + + describe '#table_name' do + it 'returns the owner_table' do + expect(sequence_sql_adapter.table_name).to eq(owner_table) + end + end + + describe '#column_owner' do + context 'when owner_schema, owner_table, and owner_column are all present' do + it 'returns fully qualified column reference' do + expect(sequence_sql_adapter.column_owner).to eq('app_schema.users.id') + end + end + + context 'when only owner_table and owner_column are present' do + let(:sequence_name) { nil } + let(:schema_name) { nil } + let(:owner_schema) { nil } + + it 'returns table.column format' do + expect(sequence_sql_adapter.column_owner).to eq('public.users.id') + end + end + + context 'when only owner_column is present' do + let(:owner_table) { nil } + + it 'returns nil' do + expect(sequence_sql_adapter.column_owner).to be_nil + end + end + + context 'when no owner information is present' do + let(:owner_table) { nil } + let(:owner_column) { nil } + + it 'returns nil' do + expect(sequence_sql_adapter.column_owner).to be_nil + end + end + end + + describe '#schema' do + context 'when schema_name is present' do + it 'returns schema_name' do + expect(sequence_sql_adapter.schema).to eq(schema_name) + end + end + + context 'when schema_name is nil but owner_schema is present' do + let(:schema_name) { nil } + + it 'returns owner_schema' do + expect(sequence_sql_adapter.schema).to eq(owner_schema) + end + end + + context 'when both schema_name and owner_schema are nil' do + let(:schema_name) { nil } + let(:owner_schema) { nil } + + it 'returns default public schema' do + expect(sequence_sql_adapter.schema).to eq('public') + end + end + + context 'when schema_name is present and owner_schema is also present' do + it 'prioritizes schema_name over owner_schema' do + expect(sequence_sql_adapter.schema).to eq(schema_name) + end + end + end + + describe '#to_s' do + it 'returns a string representation' do + expect(sequence_sql_adapter.to_s).to eq('SequenceStructureSqlAdapter(public.users_id_seq -> app_schema.users.id)') + end + end + + describe '#inspect' do + it 'returns an inspect string with object_id' do + result = sequence_sql_adapter.inspect + expect(result).to match(/^# app_schema\.users\.id>$/) + end + end +end diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser_spec.rb new file mode 100644 index 00000000000000..23a82bfb6fe087 --- /dev/null +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/sources/sequence_structure_sql_parser_spec.rb @@ -0,0 +1,247 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# rubocop:disable Rails/SquishedSQLHeredocs -- This gem does not depend on Rails +RSpec.describe Gitlab::Schema::Validation::Sources::SequenceStructureSqlParser, feature_category: :database do + let(:default_schema_name) { 'public' } + + subject(:parser) { described_class.new(parsed_structure, default_schema_name) } + + describe '#execute' do + let(:parsed_structure) { PgQuery.parse(sql) } + + context 'with CREATE SEQUENCE statements' do + where(:sql, :expected_sequences) do + [ + [ + 'CREATE SEQUENCE public.web_hook_logs_id_seq;', + { 'public.web_hook_logs_id_seq' => { sequence_name: 'web_hook_logs_id_seq', schema_name: 'public' } } + ], + [ + 'CREATE SEQUENCE web_hook_logs_id_seq;', + { 'public.web_hook_logs_id_seq' => { sequence_name: 'web_hook_logs_id_seq', schema_name: 'public' } } + ], + [ + 'CREATE SEQUENCE custom_schema.test_seq;', + { 'custom_schema.test_seq' => { sequence_name: 'test_seq', schema_name: 'custom_schema' } } + ] + ] + end + + with_them do + it 'creates sequences with correct attributes' do + result = parser.execute + + expected_sequences.each do |full_name, expected_attrs| + expect(result).to have_key(full_name) + sequence = result[full_name] + expect(sequence).to be_a(Gitlab::Schema::Validation::Adapters::SequenceStructureSqlAdapter) + expect(sequence.sequence_name).to eq(expected_attrs[:sequence_name]) + expect(sequence.schema_name).to eq(expected_attrs[:schema_name]) + end + end + end + end + + context 'with ALTER SEQUENCE OWNED BY statements' do + let(:sql) do + <<~SQL + CREATE SEQUENCE public.ai_code_suggestion_events_id_seq; + ALTER SEQUENCE public.ai_code_suggestion_events_id_seq OWNED BY ai_code_suggestion_events.id; + SQL + end + + it 'sets ownership information' do + result = parser.execute + sequence = result['public.ai_code_suggestion_events_id_seq'] + + expect(sequence.sequence_name).to eq('ai_code_suggestion_events_id_seq') + expect(sequence.schema_name).to eq('public') + expect(sequence.owner_table).to eq('ai_code_suggestion_events') + expect(sequence.owner_column).to eq('id') + expect(sequence.owner_schema).to eq('public') + end + end + + context 'with ALTER SEQUENCE OWNED BY with schema.table.column format' do + let(:sql) do + <<~SQL + CREATE SEQUENCE public.test_seq; + ALTER SEQUENCE public.test_seq OWNED BY custom_schema.test_table.test_column; + SQL + end + + it 'sets ownership information with custom schema' do + result = parser.execute + sequence = result['public.test_seq'] + + expect(sequence.owner_table).to eq('test_table') + expect(sequence.owner_column).to eq('test_column') + expect(sequence.owner_schema).to eq('custom_schema') + end + end + + context 'with ALTER TABLE SET DEFAULT nextval statements' do + # rubocop:disable Layout/LineLength -- Long SQL statements are unavoidable + where(:sql, :parsed_sequence_name, :expected_sequence_name, :expected_owner_table, :expected_owner_column) do + [ + [ + "CREATE SEQUENCE public.web_hook_logs_id_seq; + ALTER TABLE ONLY public.web_hook_logs ALTER COLUMN id SET DEFAULT nextval('web_hook_logs_id_seq'::regclass);", + 'public.web_hook_logs_id_seq', + 'web_hook_logs_id_seq', + 'web_hook_logs', + 'id' + ], + [ + "CREATE SEQUENCE public.issues_id_seq; + ALTER TABLE public.issues ALTER COLUMN id SET DEFAULT nextval('public.issues_id_seq'::regclass);", + 'public.issues_id_seq', + 'issues_id_seq', + 'issues', + 'id' + ], + [ + "CREATE SEQUENCE public.test_seq; + ALTER TABLE custom_schema.test_table ALTER COLUMN test_id SET DEFAULT nextval('test_seq'::regclass);", + 'public.test_seq', + 'test_seq', + 'test_table', + 'test_id' + ] + ] + # rubocop:enable Layout/LineLength + end + + with_them do + it 'extracts sequence ownership from ALTER TABLE statements' do + result = parser.execute + sequence = result[parsed_sequence_name] + + expect(sequence).to be_a(Gitlab::Schema::Validation::Adapters::SequenceStructureSqlAdapter) + expect(sequence.sequence_name).to eq(expected_sequence_name) + expect(sequence.owner_table).to eq(expected_owner_table) + expect(sequence.owner_column).to eq(expected_owner_column) + end + end + end + + context 'with combined CREATE and ALTER statements' do + let(:sql) do + <<~SQL + CREATE SEQUENCE public.web_hook_logs_id_seq; + ALTER TABLE ONLY public.web_hook_logs ALTER COLUMN id SET DEFAULT nextval('web_hook_logs_id_seq'::regclass); + ALTER SEQUENCE public.web_hook_logs_id_seq OWNED BY web_hook_logs.id; + SQL + end + + it 'processes all statements and merges information' do + result = parser.execute + sequence = result['public.web_hook_logs_id_seq'] + + expect(sequence.sequence_name).to eq('web_hook_logs_id_seq') + expect(sequence.schema_name).to eq('public') + expect(sequence.owner_table).to eq('web_hook_logs') + expect(sequence.owner_column).to eq('id') + expect(sequence.owner_schema).to eq('public') + end + end + + context 'with multiple sequences' do + let(:sql) do + <<~SQL + CREATE SEQUENCE public.users_id_seq; + CREATE SEQUENCE public.projects_id_seq; + ALTER TABLE ONLY public.users ALTER COLUMN id SET DEFAULT nextval('users_id_seq'::regclass); + ALTER TABLE ONLY public.projects ALTER COLUMN id SET DEFAULT nextval('projects_id_seq'::regclass); + SQL + end + + it 'processes multiple sequences correctly' do + result = parser.execute + + expect(result).to have_key('public.users_id_seq') + expect(result).to have_key('public.projects_id_seq') + + users_seq = result['public.users_id_seq'] + projects_seq = result['public.projects_id_seq'] + + expect(users_seq.owner_table).to eq('users') + expect(projects_seq.owner_table).to eq('projects') + end + end + + context 'with ALTER SEQUENCE on non-existent sequence' do + let(:sql) do + <<~SQL + ALTER SEQUENCE public.non_existent_seq OWNED BY test_table.id; + SQL + end + + it 'warns about missing sequence' do + expect(parser).to receive(:warn).with( + 'Could not find sequence public.non_existent_seq for ALTER SEQUENCE command') + parser.execute + end + end + + context 'with non-sequence statements' do + let(:sql) do + <<~SQL + CREATE TABLE public.test_table (id integer); + CREATE INDEX idx_test ON public.test_table (id); + ALTER TABLE public.test_table ADD COLUMN name varchar(255); + SQL + end + + it 'ignores non-sequence statements' do + result = parser.execute + expect(result).to be_empty + end + end + + context 'with empty SQL' do + let(:sql) { '' } + + it 'retuSQL.squishSQL.squishrns empty hash' do + result = parser.execute + expect(result).to eq({}) + end + end + + context 'with complex nextval expressions' do + let(:sql) do + <<~SQL + CREATE SEQUENCE public.test_seq; + ALTER TABLE public.test_table ALTER COLUMN id SET DEFAULT nextval(('test_seq'::text)::regclass); + SQL + end + + it 'extracts sequence name from complex expressions' do + result = parser.execute + sequence = result['public.test_seq'] + + expect(sequence.sequence_name).to eq('test_seq') + expect(sequence.owner_table).to eq('test_table') + expect(sequence.owner_column).to eq('id') + end + end + end + + describe 'default schema handling' do + context 'with custom default schema' do + let(:default_schema_name) { 'custom_default' } + let(:sql) { 'CREATE SEQUENCE test_seq;' } + let(:parsed_structure) { PgQuery.parse(sql) } + + it 'uses custom default schema' do + result = parser.execute + sequence = result['custom_default.test_seq'] + + expect(sequence.schema_name).to eq('custom_default') + end + end + end +end +# rubocop:enable Rails/SquishedSQLHeredocs diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb index 50be1f1b373264..d4e507f02e7915 100644 --- a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb @@ -24,6 +24,7 @@ Gitlab::Schema::Validation::Validators::MissingIndexes, Gitlab::Schema::Validation::Validators::MissingTriggers, Gitlab::Schema::Validation::Validators::MissingForeignKeys, + Gitlab::Schema::Validation::Validators::MissingSequences, Gitlab::Schema::Validation::Validators::DifferentDefinitionTables, Gitlab::Schema::Validation::Validators::DifferentDefinitionIndexes, Gitlab::Schema::Validation::Validators::DifferentDefinitionTriggers, diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb new file mode 100644 index 00000000000000..842de750216918 --- /dev/null +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Schema::Validation::Validators::MissingSequences, feature_category: :database do + missing_sequences = %w[ + public.missing_sequence + public.shared_audit_event_id_seq + public.abuse_events_id_seq + ] + + include_examples 'sequence validators', described_class, missing_sequences +end diff --git a/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb b/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb new file mode 100644 index 00000000000000..aae7013879dd58 --- /dev/null +++ b/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'sequence validators' do |validator, expected_result| + let(:structure_file_path) { 'spec/fixtures/structure.sql' } + let(:database_sequences) do + %w[ + wrong_sequence + extra_sequence + shared_audit_event_id_seq + ] + end + + let(:inconsistency_type) { validator.name } + let(:connection_class) { class_double(Class, name: 'ActiveRecord::ConnectionAdapters::PostgreSQLAdapter') } + # rubocop:disable RSpec/VerifiedDoubleReference + let(:connection) do + instance_double('connection', class: connection_class, current_schema: 'public') + end + # rubocop:enable RSpec/VerifiedDoubleReference + + let(:schema) { 'public' } + let(:database) { Gitlab::Schema::Validation::Sources::Database.new(connection) } + let(:structure_file) { Gitlab::Schema::Validation::Sources::StructureSql.new(structure_file_path, schema) } + + before do + allow(database).to receive(:sequence_exists?) do |sequence_name| + database_sequences.include?(sequence_name) + end + end + + subject(:result) { validator.new(structure_file, database).execute } + + it 'returns sequence inconsistencies' do + expect(result.map(&:object_name)).to match_array(expected_result) + expect(result.map(&:type)).to all(eql inconsistency_type) + end +end -- GitLab