diff --git a/app/models/concerns/use_sql_function_for_primary_key_lookups.rb b/app/models/concerns/use_sql_function_for_primary_key_lookups.rb new file mode 100644 index 0000000000000000000000000000000000000000..c3ca3cfc03849674b5feeacbdcdd0ebde30cb22c --- /dev/null +++ b/app/models/concerns/use_sql_function_for_primary_key_lookups.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module UseSqlFunctionForPrimaryKeyLookups + extend ActiveSupport::Concern + + class_methods do + def find(*args) + return super unless Feature.enabled?(:use_sql_functions_for_primary_key_lookups, Feature.current_request) + return super unless args.one? + return super if block_given? || primary_key.nil? || scope_attributes? + + return_array = false + id = args.first + + if id.is_a?(Array) + return super if id.many? + + return_array = true + + id = id.first + end + + return super if id.nil? || (id.is_a?(String) && !id.number?) + + from_clause = "find_#{table_name}_by_id(?) #{quoted_table_name}" + filter_empty_row = "#{quoted_table_name}.#{connection.quote_column_name(primary_key)} IS NOT NULL" + query = from(from_clause).where(filter_empty_row).limit(1).to_sql + # Using find_by_sql so we get query cache working + record = find_by_sql([query, id]).first + + unless record + message = "Couldn't find #{name} with '#{primary_key}'=#{id}" + raise(ActiveRecord::RecordNotFound.new(message, name, primary_key, id)) + end + + return_array ? [record] : record + end + end +end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 733b89fcaf254de0fe5c4fe60fa76f2ba9da3b13..6414d6a0081c63d99f1e4bbfdc1908eef01f42f7 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -18,6 +18,7 @@ class Namespace < ApplicationRecord include Referable include CrossDatabaseIgnoredTables include IgnorableColumns + include UseSqlFunctionForPrimaryKeyLookups ignore_column :unlock_membership_to_ldap, remove_with: '16.7', remove_after: '2023-11-16' diff --git a/app/models/project.rb b/app/models/project.rb index f7e994d4beb5f98a7b55dafbb5241a634e3a531f..72d6616979804114982dbaca59e12e3e1a698ed7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -45,6 +45,7 @@ class Project < ApplicationRecord include UpdatedAtFilterable include IgnorableColumns include CrossDatabaseIgnoredTables + include UseSqlFunctionForPrimaryKeyLookups ignore_column :emails_disabled, remove_with: '16.3', remove_after: '2023-08-22' diff --git a/app/models/user.rb b/app/models/user.rb index fdc0b531521bc1505edf5d788daafa7480f52c40..3b8588dae32906af0f06b9563e246db847bd6d03 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -32,6 +32,7 @@ class User < MainClusterwide::ApplicationRecord include EachBatch include CrossDatabaseIgnoredTables include IgnorableColumns + include UseSqlFunctionForPrimaryKeyLookups ignore_column %i[ email_opted_in diff --git a/config/feature_flags/development/use_sql_functions_for_primary_key_lookups.yml b/config/feature_flags/development/use_sql_functions_for_primary_key_lookups.yml new file mode 100644 index 0000000000000000000000000000000000000000..c8ee2894aef7c800c2a70e84ef397ef5a65f31f9 --- /dev/null +++ b/config/feature_flags/development/use_sql_functions_for_primary_key_lookups.yml @@ -0,0 +1,8 @@ +--- +name: use_sql_functions_for_primary_key_lookups +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135196 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/429479 +milestone: '16.6' +type: development +group: group::optimize +default_enabled: false diff --git a/db/migrate/20231026050554_add_functions_for_primary_key_lookup.rb b/db/migrate/20231026050554_add_functions_for_primary_key_lookup.rb new file mode 100644 index 0000000000000000000000000000000000000000..ecf32f74e4b0eb7bde94b7418d4875970f8cdfbe --- /dev/null +++ b/db/migrate/20231026050554_add_functions_for_primary_key_lookup.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class AddFunctionsForPrimaryKeyLookup < Gitlab::Database::Migration[2.2] + milestone '16.6' + + TABLES = %i[users namespaces projects].freeze + + def up + TABLES.each do |table| + execute <<~SQL + CREATE OR REPLACE FUNCTION find_#{table}_by_id(#{table}_id bigint) + RETURNS #{table} AS $$ + BEGIN + return (SELECT #{table} FROM #{table} WHERE id = #{table}_id LIMIT 1); + END; + $$ LANGUAGE plpgsql STABLE PARALLEL SAFE COST 1; + SQL + end + end + + def down + TABLES.each do |table| + execute "DROP FUNCTION IF EXISTS find_#{table}_by_id" + end + end +end diff --git a/db/schema_migrations/20231026050554 b/db/schema_migrations/20231026050554 new file mode 100644 index 0000000000000000000000000000000000000000..d99dc675ab6c3b71a2dd6e293c198088ccb6cd24 --- /dev/null +++ b/db/schema_migrations/20231026050554 @@ -0,0 +1 @@ +e71f80b77121722c75125e59ec2e9c3df323b34a107304447948bed05804224c \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 043dd201d5a17cf0de88caa00bf670acb1a6523b..3e050fd0853b9e6ee5b542202f9c45b5248e0f24 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -35,6 +35,248 @@ RETURN NULL; END $$; +CREATE TABLE namespaces ( + id integer NOT NULL, + name character varying NOT NULL, + path character varying NOT NULL, + owner_id integer, + created_at timestamp without time zone, + updated_at timestamp without time zone, + type character varying DEFAULT 'User'::character varying NOT NULL, + description character varying DEFAULT ''::character varying NOT NULL, + avatar character varying, + membership_lock boolean DEFAULT false, + share_with_group_lock boolean DEFAULT false, + visibility_level integer DEFAULT 20 NOT NULL, + request_access_enabled boolean DEFAULT true NOT NULL, + ldap_sync_status character varying DEFAULT 'ready'::character varying NOT NULL, + ldap_sync_error character varying, + ldap_sync_last_update_at timestamp without time zone, + ldap_sync_last_successful_update_at timestamp without time zone, + ldap_sync_last_sync_at timestamp without time zone, + description_html text, + lfs_enabled boolean, + parent_id integer, + shared_runners_minutes_limit integer, + repository_size_limit bigint, + require_two_factor_authentication boolean DEFAULT false NOT NULL, + two_factor_grace_period integer DEFAULT 48 NOT NULL, + cached_markdown_version integer, + project_creation_level integer, + runners_token character varying, + file_template_project_id integer, + saml_discovery_token character varying, + runners_token_encrypted character varying, + custom_project_templates_group_id integer, + auto_devops_enabled boolean, + extra_shared_runners_minutes_limit integer, + last_ci_minutes_notification_at timestamp with time zone, + last_ci_minutes_usage_notification_level integer, + subgroup_creation_level integer DEFAULT 1, + emails_disabled boolean, + max_pages_size integer, + max_artifacts_size integer, + mentions_disabled boolean, + default_branch_protection smallint, + unlock_membership_to_ldap boolean, + max_personal_access_token_lifetime integer, + push_rule_id bigint, + shared_runners_enabled boolean DEFAULT true NOT NULL, + allow_descendants_override_disabled_shared_runners boolean DEFAULT false NOT NULL, + traversal_ids integer[] DEFAULT '{}'::integer[] NOT NULL, + organization_id bigint DEFAULT 1 +); + +CREATE FUNCTION find_namespaces_by_id(namespaces_id bigint) RETURNS namespaces + LANGUAGE plpgsql STABLE COST 1 PARALLEL SAFE + AS $$ +BEGIN + return (SELECT namespaces FROM namespaces WHERE id = namespaces_id LIMIT 1); +END; +$$; + +CREATE TABLE projects ( + id integer NOT NULL, + name character varying, + path character varying, + description text, + created_at timestamp without time zone, + updated_at timestamp without time zone, + creator_id integer, + namespace_id integer NOT NULL, + last_activity_at timestamp without time zone, + import_url character varying, + visibility_level integer DEFAULT 0 NOT NULL, + archived boolean DEFAULT false NOT NULL, + avatar character varying, + merge_requests_template text, + star_count integer DEFAULT 0 NOT NULL, + merge_requests_rebase_enabled boolean DEFAULT false, + import_type character varying, + import_source character varying, + approvals_before_merge integer DEFAULT 0 NOT NULL, + reset_approvals_on_push boolean DEFAULT true, + merge_requests_ff_only_enabled boolean DEFAULT false, + issues_template text, + mirror boolean DEFAULT false NOT NULL, + mirror_last_update_at timestamp without time zone, + mirror_last_successful_update_at timestamp without time zone, + mirror_user_id integer, + shared_runners_enabled boolean DEFAULT true NOT NULL, + runners_token character varying, + build_allow_git_fetch boolean DEFAULT true NOT NULL, + build_timeout integer DEFAULT 3600 NOT NULL, + mirror_trigger_builds boolean DEFAULT false NOT NULL, + pending_delete boolean DEFAULT false, + public_builds boolean DEFAULT true NOT NULL, + last_repository_check_failed boolean, + last_repository_check_at timestamp without time zone, + only_allow_merge_if_pipeline_succeeds boolean DEFAULT false NOT NULL, + has_external_issue_tracker boolean, + repository_storage character varying DEFAULT 'default'::character varying NOT NULL, + repository_read_only boolean, + request_access_enabled boolean DEFAULT true NOT NULL, + has_external_wiki boolean, + ci_config_path character varying, + lfs_enabled boolean, + description_html text, + only_allow_merge_if_all_discussions_are_resolved boolean, + repository_size_limit bigint, + printing_merge_request_link_enabled boolean DEFAULT true NOT NULL, + auto_cancel_pending_pipelines integer DEFAULT 1 NOT NULL, + service_desk_enabled boolean DEFAULT true, + cached_markdown_version integer, + delete_error text, + last_repository_updated_at timestamp without time zone, + disable_overriding_approvers_per_merge_request boolean, + storage_version smallint, + resolve_outdated_diff_discussions boolean, + remote_mirror_available_overridden boolean, + only_mirror_protected_branches boolean, + pull_mirror_available_overridden boolean, + jobs_cache_index integer, + external_authorization_classification_label character varying, + mirror_overwrites_diverged_branches boolean, + pages_https_only boolean DEFAULT true, + external_webhook_token character varying, + packages_enabled boolean, + merge_requests_author_approval boolean DEFAULT false, + pool_repository_id bigint, + runners_token_encrypted character varying, + bfg_object_map character varying, + detected_repository_languages boolean, + merge_requests_disable_committers_approval boolean, + require_password_to_approve boolean, + emails_disabled boolean, + max_pages_size integer, + max_artifacts_size integer, + pull_mirror_branch_prefix character varying(50), + remove_source_branch_after_merge boolean, + marked_for_deletion_at date, + marked_for_deletion_by_user_id integer, + autoclose_referenced_issues boolean, + suggestion_commit_message character varying(255), + project_namespace_id bigint, + hidden boolean DEFAULT false NOT NULL, + organization_id bigint DEFAULT 1 +); + +CREATE FUNCTION find_projects_by_id(projects_id bigint) RETURNS projects + LANGUAGE plpgsql STABLE COST 1 PARALLEL SAFE + AS $$ +BEGIN + return (SELECT projects FROM projects WHERE id = projects_id LIMIT 1); +END; +$$; + +CREATE TABLE users ( + id integer NOT NULL, + email character varying DEFAULT ''::character varying NOT NULL, + encrypted_password character varying DEFAULT ''::character varying NOT NULL, + reset_password_token character varying, + reset_password_sent_at timestamp without time zone, + remember_created_at timestamp without time zone, + sign_in_count integer DEFAULT 0, + current_sign_in_at timestamp without time zone, + last_sign_in_at timestamp without time zone, + current_sign_in_ip character varying, + last_sign_in_ip character varying, + created_at timestamp without time zone, + updated_at timestamp without time zone, + name character varying, + admin boolean DEFAULT false NOT NULL, + projects_limit integer NOT NULL, + failed_attempts integer DEFAULT 0, + locked_at timestamp without time zone, + username character varying, + can_create_group boolean DEFAULT true NOT NULL, + can_create_team boolean DEFAULT true NOT NULL, + state character varying, + color_scheme_id integer DEFAULT 1 NOT NULL, + password_expires_at timestamp without time zone, + created_by_id integer, + last_credential_check_at timestamp without time zone, + avatar character varying, + confirmation_token character varying, + confirmed_at timestamp without time zone, + confirmation_sent_at timestamp without time zone, + unconfirmed_email character varying, + hide_no_ssh_key boolean DEFAULT false, + admin_email_unsubscribed_at timestamp without time zone, + notification_email character varying, + hide_no_password boolean DEFAULT false, + password_automatically_set boolean DEFAULT false, + encrypted_otp_secret character varying, + encrypted_otp_secret_iv character varying, + encrypted_otp_secret_salt character varying, + otp_required_for_login boolean DEFAULT false NOT NULL, + otp_backup_codes text, + public_email character varying, + dashboard integer DEFAULT 0, + project_view integer DEFAULT 2, + consumed_timestep integer, + layout integer DEFAULT 0, + hide_project_limit boolean DEFAULT false, + note text, + unlock_token character varying, + otp_grace_period_started_at timestamp without time zone, + external boolean DEFAULT false, + incoming_email_token character varying, + auditor boolean DEFAULT false NOT NULL, + require_two_factor_authentication_from_group boolean DEFAULT false NOT NULL, + two_factor_grace_period integer DEFAULT 48 NOT NULL, + last_activity_on date, + notified_of_own_activity boolean DEFAULT false, + preferred_language character varying, + theme_id smallint, + accepted_term_id integer, + feed_token character varying, + private_profile boolean DEFAULT false NOT NULL, + roadmap_layout smallint, + include_private_contributions boolean, + commit_email character varying, + group_view integer, + managing_group_id integer, + first_name character varying(255), + last_name character varying(255), + static_object_token character varying(255), + role smallint, + user_type smallint DEFAULT 0, + static_object_token_encrypted text, + otp_secret_expires_at timestamp with time zone, + onboarding_in_progress boolean DEFAULT false NOT NULL, + CONSTRAINT check_0dd5948e38 CHECK ((user_type IS NOT NULL)), + CONSTRAINT check_7bde697e8e CHECK ((char_length(static_object_token_encrypted) <= 255)) +); + +CREATE FUNCTION find_users_by_id(users_id bigint) RETURNS users + LANGUAGE plpgsql STABLE COST 1 PARALLEL SAFE + AS $$ +BEGIN + return (SELECT users FROM users WHERE id = users_id LIMIT 1); +END; +$$; + CREATE FUNCTION gitlab_schema_prevent_write() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -19192,58 +19434,6 @@ CREATE SEQUENCE namespace_statistics_id_seq ALTER SEQUENCE namespace_statistics_id_seq OWNED BY namespace_statistics.id; -CREATE TABLE namespaces ( - id integer NOT NULL, - name character varying NOT NULL, - path character varying NOT NULL, - owner_id integer, - created_at timestamp without time zone, - updated_at timestamp without time zone, - type character varying DEFAULT 'User'::character varying NOT NULL, - description character varying DEFAULT ''::character varying NOT NULL, - avatar character varying, - membership_lock boolean DEFAULT false, - share_with_group_lock boolean DEFAULT false, - visibility_level integer DEFAULT 20 NOT NULL, - request_access_enabled boolean DEFAULT true NOT NULL, - ldap_sync_status character varying DEFAULT 'ready'::character varying NOT NULL, - ldap_sync_error character varying, - ldap_sync_last_update_at timestamp without time zone, - ldap_sync_last_successful_update_at timestamp without time zone, - ldap_sync_last_sync_at timestamp without time zone, - description_html text, - lfs_enabled boolean, - parent_id integer, - shared_runners_minutes_limit integer, - repository_size_limit bigint, - require_two_factor_authentication boolean DEFAULT false NOT NULL, - two_factor_grace_period integer DEFAULT 48 NOT NULL, - cached_markdown_version integer, - project_creation_level integer, - runners_token character varying, - file_template_project_id integer, - saml_discovery_token character varying, - runners_token_encrypted character varying, - custom_project_templates_group_id integer, - auto_devops_enabled boolean, - extra_shared_runners_minutes_limit integer, - last_ci_minutes_notification_at timestamp with time zone, - last_ci_minutes_usage_notification_level integer, - subgroup_creation_level integer DEFAULT 1, - emails_disabled boolean, - max_pages_size integer, - max_artifacts_size integer, - mentions_disabled boolean, - default_branch_protection smallint, - unlock_membership_to_ldap boolean, - max_personal_access_token_lifetime integer, - push_rule_id bigint, - shared_runners_enabled boolean DEFAULT true NOT NULL, - allow_descendants_override_disabled_shared_runners boolean DEFAULT false NOT NULL, - traversal_ids integer[] DEFAULT '{}'::integer[] NOT NULL, - organization_id bigint DEFAULT 1 -); - CREATE SEQUENCE namespaces_id_seq START WITH 1 INCREMENT BY 1 @@ -21900,92 +22090,6 @@ CREATE SEQUENCE project_wiki_repositories_id_seq ALTER SEQUENCE project_wiki_repositories_id_seq OWNED BY project_wiki_repositories.id; -CREATE TABLE projects ( - id integer NOT NULL, - name character varying, - path character varying, - description text, - created_at timestamp without time zone, - updated_at timestamp without time zone, - creator_id integer, - namespace_id integer NOT NULL, - last_activity_at timestamp without time zone, - import_url character varying, - visibility_level integer DEFAULT 0 NOT NULL, - archived boolean DEFAULT false NOT NULL, - avatar character varying, - merge_requests_template text, - star_count integer DEFAULT 0 NOT NULL, - merge_requests_rebase_enabled boolean DEFAULT false, - import_type character varying, - import_source character varying, - approvals_before_merge integer DEFAULT 0 NOT NULL, - reset_approvals_on_push boolean DEFAULT true, - merge_requests_ff_only_enabled boolean DEFAULT false, - issues_template text, - mirror boolean DEFAULT false NOT NULL, - mirror_last_update_at timestamp without time zone, - mirror_last_successful_update_at timestamp without time zone, - mirror_user_id integer, - shared_runners_enabled boolean DEFAULT true NOT NULL, - runners_token character varying, - build_allow_git_fetch boolean DEFAULT true NOT NULL, - build_timeout integer DEFAULT 3600 NOT NULL, - mirror_trigger_builds boolean DEFAULT false NOT NULL, - pending_delete boolean DEFAULT false, - public_builds boolean DEFAULT true NOT NULL, - last_repository_check_failed boolean, - last_repository_check_at timestamp without time zone, - only_allow_merge_if_pipeline_succeeds boolean DEFAULT false NOT NULL, - has_external_issue_tracker boolean, - repository_storage character varying DEFAULT 'default'::character varying NOT NULL, - repository_read_only boolean, - request_access_enabled boolean DEFAULT true NOT NULL, - has_external_wiki boolean, - ci_config_path character varying, - lfs_enabled boolean, - description_html text, - only_allow_merge_if_all_discussions_are_resolved boolean, - repository_size_limit bigint, - printing_merge_request_link_enabled boolean DEFAULT true NOT NULL, - auto_cancel_pending_pipelines integer DEFAULT 1 NOT NULL, - service_desk_enabled boolean DEFAULT true, - cached_markdown_version integer, - delete_error text, - last_repository_updated_at timestamp without time zone, - disable_overriding_approvers_per_merge_request boolean, - storage_version smallint, - resolve_outdated_diff_discussions boolean, - remote_mirror_available_overridden boolean, - only_mirror_protected_branches boolean, - pull_mirror_available_overridden boolean, - jobs_cache_index integer, - external_authorization_classification_label character varying, - mirror_overwrites_diverged_branches boolean, - pages_https_only boolean DEFAULT true, - external_webhook_token character varying, - packages_enabled boolean, - merge_requests_author_approval boolean DEFAULT false, - pool_repository_id bigint, - runners_token_encrypted character varying, - bfg_object_map character varying, - detected_repository_languages boolean, - merge_requests_disable_committers_approval boolean, - require_password_to_approve boolean, - emails_disabled boolean, - max_pages_size integer, - max_artifacts_size integer, - pull_mirror_branch_prefix character varying(50), - remove_source_branch_after_merge boolean, - marked_for_deletion_at date, - marked_for_deletion_by_user_id integer, - autoclose_referenced_issues boolean, - suggestion_commit_message character varying(255), - project_namespace_id bigint, - hidden boolean DEFAULT false NOT NULL, - organization_id bigint DEFAULT 1 -); - CREATE SEQUENCE projects_id_seq START WITH 1 INCREMENT BY 1 @@ -24396,86 +24500,6 @@ CREATE SEQUENCE user_synced_attributes_metadata_id_seq ALTER SEQUENCE user_synced_attributes_metadata_id_seq OWNED BY user_synced_attributes_metadata.id; -CREATE TABLE users ( - id integer NOT NULL, - email character varying DEFAULT ''::character varying NOT NULL, - encrypted_password character varying DEFAULT ''::character varying NOT NULL, - reset_password_token character varying, - reset_password_sent_at timestamp without time zone, - remember_created_at timestamp without time zone, - sign_in_count integer DEFAULT 0, - current_sign_in_at timestamp without time zone, - last_sign_in_at timestamp without time zone, - current_sign_in_ip character varying, - last_sign_in_ip character varying, - created_at timestamp without time zone, - updated_at timestamp without time zone, - name character varying, - admin boolean DEFAULT false NOT NULL, - projects_limit integer NOT NULL, - failed_attempts integer DEFAULT 0, - locked_at timestamp without time zone, - username character varying, - can_create_group boolean DEFAULT true NOT NULL, - can_create_team boolean DEFAULT true NOT NULL, - state character varying, - color_scheme_id integer DEFAULT 1 NOT NULL, - password_expires_at timestamp without time zone, - created_by_id integer, - last_credential_check_at timestamp without time zone, - avatar character varying, - confirmation_token character varying, - confirmed_at timestamp without time zone, - confirmation_sent_at timestamp without time zone, - unconfirmed_email character varying, - hide_no_ssh_key boolean DEFAULT false, - admin_email_unsubscribed_at timestamp without time zone, - notification_email character varying, - hide_no_password boolean DEFAULT false, - password_automatically_set boolean DEFAULT false, - encrypted_otp_secret character varying, - encrypted_otp_secret_iv character varying, - encrypted_otp_secret_salt character varying, - otp_required_for_login boolean DEFAULT false NOT NULL, - otp_backup_codes text, - public_email character varying, - dashboard integer DEFAULT 0, - project_view integer DEFAULT 2, - consumed_timestep integer, - layout integer DEFAULT 0, - hide_project_limit boolean DEFAULT false, - note text, - unlock_token character varying, - otp_grace_period_started_at timestamp without time zone, - external boolean DEFAULT false, - incoming_email_token character varying, - auditor boolean DEFAULT false NOT NULL, - require_two_factor_authentication_from_group boolean DEFAULT false NOT NULL, - two_factor_grace_period integer DEFAULT 48 NOT NULL, - last_activity_on date, - notified_of_own_activity boolean DEFAULT false, - preferred_language character varying, - theme_id smallint, - accepted_term_id integer, - feed_token character varying, - private_profile boolean DEFAULT false NOT NULL, - roadmap_layout smallint, - include_private_contributions boolean, - commit_email character varying, - group_view integer, - managing_group_id integer, - first_name character varying(255), - last_name character varying(255), - static_object_token character varying(255), - role smallint, - user_type smallint DEFAULT 0, - static_object_token_encrypted text, - otp_secret_expires_at timestamp with time zone, - onboarding_in_progress boolean DEFAULT false NOT NULL, - CONSTRAINT check_0dd5948e38 CHECK ((user_type IS NOT NULL)), - CONSTRAINT check_7bde697e8e CHECK ((char_length(static_object_token_encrypted) <= 255)) -); - CREATE SEQUENCE users_id_seq START WITH 1 INCREMENT BY 1 diff --git a/spec/models/concerns/use_sql_function_for_primary_key_lookups_spec.rb b/spec/models/concerns/use_sql_function_for_primary_key_lookups_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f6f53c9aad50b6dd67b65bc8639cc5e1a1836844 --- /dev/null +++ b/spec/models/concerns/use_sql_function_for_primary_key_lookups_spec.rb @@ -0,0 +1,181 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe UseSqlFunctionForPrimaryKeyLookups, feature_category: :groups_and_projects do + let_it_be(:project) { create(:project) } + let_it_be(:another_project) { create(:project) } + + let(:model) do + Class.new(ApplicationRecord) do + self.table_name = :projects + + include UseSqlFunctionForPrimaryKeyLookups + end + end + + context 'when the use_sql_functions_for_primary_key_lookups FF is on' do + before do + stub_feature_flags(use_sql_functions_for_primary_key_lookups: true) + end + + it 'loads the correct record' do + expect(model.find(project.id).id).to eq(project.id) + end + + it 'uses the fuction-based finder query' do + query = <<~SQL + SELECT "projects".* FROM find_projects_by_id(#{project.id})#{' '} + "projects" WHERE ("projects"."id" IS NOT NULL) LIMIT 1 + SQL + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id) }.log + + expect(query_log).to match_array(include(query.tr("\n", ''))) + end + + it 'uses query cache', :use_sql_query_cache do + query = <<~SQL + SELECT "projects".* FROM find_projects_by_id(#{project.id})#{' '} + "projects" WHERE ("projects"."id" IS NOT NULL) LIMIT 1 + SQL + + recorder = ActiveRecord::QueryRecorder.new do + model.find(project.id) + model.find(project.id) + model.find(project.id) + end + + expect(recorder.data.each_value.first[:count]).to eq(1) + expect(recorder.cached).to include(query.tr("\n", '')) + end + + context 'when the model has ignored columns' do + around do |example| + model.ignored_columns = %i[path] + example.run + model.ignored_columns = [] + end + + it 'enumerates the column names' do + column_list = model.columns.map do |column| + %("projects"."#{column.name}") + end.join(', ') + + expect(column_list).not_to include(%("projects"."path")) + + query = <<~SQL + SELECT #{column_list} FROM find_projects_by_id(#{project.id})#{' '} + "projects" WHERE ("projects"."id" IS NOT NULL) LIMIT 1 + SQL + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id) }.log + + expect(query_log).to match_array(include(query.tr("\n", ''))) + end + end + + context 'when there are scope attributes' do + let(:scoped_model) do + Class.new(model) do + default_scope { where.not(path: nil) } # rubocop: disable Cop/DefaultScope -- Needed for testing a specific case + end + end + + it 'loads the correct record' do + expect(scoped_model.find(project.id).id).to eq(project.id) + end + + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { scoped_model.find(project.id) }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + + context 'when there are multiple arguments' do + it 'loads the correct records' do + expect(model.find(project.id, another_project.id).map(&:id)).to match_array([project.id, another_project.id]) + end + + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id, another_project.id) }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + + context 'when there is block given' do + it 'loads the correct records' do + expect(model.find(0) { |p| p.path == project.path }.id).to eq(project.id) + end + + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find(0) { |p| p.path == project.path } }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + + context 'when there is no primary key defined' do + let(:model_without_pk) do + Class.new(model) do + def self.primary_key + nil + end + end + end + + it 'raises ActiveRecord::UnknownPrimaryKey' do + expect { model_without_pk.find(0) }.to raise_error ActiveRecord::UnknownPrimaryKey + end + end + + context 'when id is provided as an array' do + it 'returns the correct record as an array' do + expect(model.find([project.id]).map(&:id)).to eq([project.id]) + end + + it 'does use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find([project.id]) }.log + + expect(query_log).to include(match(/find_projects_by_id/)) + end + + context 'when array has multiple elements' do + it 'does not use the function-based finder query' do + query_log = ActiveRecord::QueryRecorder.new { model.find([project.id, another_project.id]) }.log + + expect(query_log).not_to include(match(/find_projects_by_id/)) + end + end + end + + context 'when the provided id is null' do + it 'raises ActiveRecord::RecordNotFound' do + expect { model.find(nil) }.to raise_error ActiveRecord::RecordNotFound, "Couldn't find without an ID" + end + end + + context 'when the provided id is not a string that can cast to numeric' do + it 'raises ActiveRecord::RecordNotFound' do + expect { model.find('foo') }.to raise_error ActiveRecord::RecordNotFound, "Couldn't find with 'id'=foo" + end + end + end + + context 'when the use_sql_functions_for_primary_key_lookups FF is off' do + before do + stub_feature_flags(use_sql_functions_for_primary_key_lookups: false) + end + + it 'loads the correct record' do + expect(model.find(project.id).id).to eq(project.id) + end + + it 'uses the SQL-based finder query' do + expected_query = %(SELECT "projects".* FROM \"projects\" WHERE "projects"."id" = #{project.id} LIMIT 1) + query_log = ActiveRecord::QueryRecorder.new { model.find(project.id) }.log + + expect(query_log).to match_array(include(expected_query)) + end + end +end