Skip to content

Don't generate invalid SQL checking foreign keys

What does this MR do and why?

Describe in detail what your merge request does and why.

When migrating many versions of gitlab at once, it was possible to call foreign_key_exists? with a schema cache too old for rails to know about the constrained_columns column of the postgres_foreign_keys view.

This manifested with errors like PG::InvalidTextRepresentation: ERROR: malformed array literal

Fallback to the previous behavior of the method in this case, to avoid the error.

Related to #388928 (closed)

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

The docker image registry.gitlab.com/gitlab-org/quality/performance-images/gitlab-ce-performance-test:15.8.0-ce.0-arm-broken reproduces this error when run. We can verify the fix by patching this commit into it.

  1. Run the docker image gitlab-ce-performance-test:15.8.0-ce.0-arm-broken and wait for the migrations to fail with the expected error.
  2. Create the following files:

Dockerfile :

FROM registry.gitlab.com/gitlab-org/quality/performance-images/gitlab-ce-performance-test:15.8.0-ce.0-arm-broken

COPY migration_helpers.patch /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/database/migration_helpers.patch
RUN cd /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/database && patch < migration_helpers.patch

migration_helpers.patch :

--- migration_helpers.rb	2023-01-20 05:07:24.000000000 -0600
+++ migration_helpers.rb	2023-03-02 13:57:58.000000000 -0600
@@ -371,14 +371,21 @@
         # The behavior in the if block has a bug: it always returns false if the fk being checked has multiple columns.
         # This can be removed after init_schema.rb passes 20221122210711_add_columns_to_postgres_foreign_keys.rb
         # Tracking issue: https://gitlab.com/gitlab-org/gitlab/-/issues/386796
-        if ActiveRecord::Migrator.current_version < 20221122210711
+        version_too_old = ActiveRecord::Migrator.current_version < 20221122210711
+
+        # We also fall back to previous behavior if the schema cache does not include the constrained_columns column.
+        # This happens if we've migrated up past the above migration and are still in the same migration execution on
+        # a subsequent migration. In that case, rails's default handling with where clauses and array columns leads
+        # to invalid SQL generating. So we fall back to the previous behavior instead.
+        schema_cache_stale = !Gitlab::Database::PostgresForeignKey.columns_hash.key?('constrained_columns')
+        if version_too_old || schema_cache_stale
           return foreign_keys(source).any? do |foreign_key|
             tables_match?(target.to_s, foreign_key.to_table.to_s) &&
                 options_match?(foreign_key.options, options)
           end
         end
 
-        fks = Gitlab::Database::PostgresForeignKey.by_constrained_table_name(source)
+        fks = Gitlab::Database::PostgresForeignKey.by_constrained_table_name_or_identifier(source)
 
         fks = fks.by_referenced_table_name(target) if target
         fks = fks.by_name(options[:name]) if options[:name]
  1. Build and run the docker image: docker build -t gitlab-fk-error:latest && docker run -it gitlab-fk-error:latest
  2. Watch the docker image proceed through migrations successfully and start gitlab.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mitchell Nielsen

Merge request reports