From 368da3ef29462f438a8df7d864b927a329c51c25 Mon Sep 17 00:00:00 2001 From: Alex Robbin Date: Sun, 6 Sep 2020 22:04:34 -0400 Subject: [PATCH] add support for `NOT VALID` check constraints in PostgreSQL Active Record already supports adding `NOT VALID` foreign key constraints in PostgreSQL, but back in #31323 when check constraint support was initially added, the ability to add a check constraint and validate it separately was not included. This adds that functionality! --- activerecord/CHANGELOG.md | 4 ++ .../abstract/schema_definitions.rb | 5 +++ .../abstract/schema_statements.rb | 5 +++ .../postgresql/schema_creation.rb | 4 ++ .../postgresql/schema_statements.rb | 18 ++++++++- .../cases/migration/check_constraint_test.rb | 37 +++++++++++++++++++ 6 files changed, 71 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 77b94e098d..c404ce7b6d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add support for check constraints that are `NOT VALID` via `validate: false` (PostgreSQL-only). + + *Alex Robbin* + * Ensure the default configuration is considered primary or first for an environment If a multiple database application provides a configuration named primary, that will be treated as default. In applications that do not have a primary entry, the default database configuration will be the first configuration for an environment. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index ddd1b6567a..68b718af22 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -132,6 +132,11 @@ module ActiveRecord options[:name] end + def validate? + options.fetch(:validate, true) + end + alias validated? validate? + def export_name_on_schema_dump? !ActiveRecord::SchemaDumper.chk_ignore_pattern.match?(name) if name end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 18de47c09f..9c40a30e9d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1142,6 +1142,11 @@ module ActiveRecord # # ALTER TABLE "products" ADD CONSTRAINT price_check CHECK (price > 0) # + # The +options+ hash can include the following keys: + # [:name] + # The constraint name. Defaults to chk_rails_. + # [:validate] + # (PostgreSQL only) Specify whether or not the constraint should be validated. Defaults to +true+. def add_check_constraint(table_name, expression, **options) return unless supports_check_constraints? diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb index c52baf5681..ad84b30a78 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb @@ -13,6 +13,10 @@ module ActiveRecord super.dup.tap { |sql| sql << " NOT VALID" unless o.validate? } end + def visit_CheckConstraintDefinition(o) + super.dup.tap { |sql| sql << " NOT VALID" unless o.validate? } + end + def visit_ValidateConstraint(name) "VALIDATE CONSTRAINT #{quote_column_name(name)}" end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index c07d1fdc40..136cfc89e5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -523,7 +523,7 @@ module ActiveRecord scope = quoted_scope(table_name) check_info = exec_query(<<-SQL, "SCHEMA") - SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef + SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.convalidated AS valid FROM pg_constraint c JOIN pg_class t ON c.conrelid = t.oid WHERE c.contype = 'c' @@ -532,7 +532,8 @@ module ActiveRecord check_info.map do |row| options = { - name: row["conname"] + name: row["conname"], + validate: row["valid"] } expression = row["constraintdef"][/CHECK \({2}(.+)\){2}/, 1] @@ -632,6 +633,19 @@ module ActiveRecord validate_constraint from_table, fk_name_to_validate end + # Validates the given check constraint. + # + # validate_check_constraint :products, name: "price_check" + # + # The +options+ hash accepts the same keys as add_check_constraint[rdoc-ref:ConnectionAdapters::SchemaStatements#add_check_constraint]. + def validate_check_constraint(table_name, **options) + return unless supports_validate_constraints? + + chk_name_to_validate = check_constraint_for!(table_name, **options).name + + validate_constraint table_name, chk_name_to_validate + end + private def schema_creation PostgreSQL::SchemaCreation.new(self) diff --git a/activerecord/test/cases/migration/check_constraint_test.rb b/activerecord/test/cases/migration/check_constraint_test.rb index dfe23fa2ea..dd96f5c98f 100644 --- a/activerecord/test/cases/migration/check_constraint_test.rb +++ b/activerecord/test/cases/migration/check_constraint_test.rb @@ -64,6 +64,43 @@ if ActiveRecord::Base.connection.supports_check_constraints? end end + if ActiveRecord::Base.connection.supports_validate_constraints? + def test_not_valid_check_constraint + Trade.create(quantity: -1) + + @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check", validate: false + + assert_raises(ActiveRecord::StatementInvalid) do + Trade.create(quantity: -1) + end + end + + def test_validate_check_constraint_by_name + @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check", validate: false + assert_not_predicate @connection.check_constraints("trades").first, :validated? + + @connection.validate_check_constraint :trades, name: "quantity_check" + assert_predicate @connection.check_constraints("trades").first, :validated? + end + + def test_validate_non_existing_check_constraint_raises + assert_raises ArgumentError do + @connection.validate_check_constraint :trades, name: "quantity_check" + end + end + else + # Check constraint should still be created, but should not be invalid + def test_add_invalid_check_constraint + @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check", validate: false + + check_constraints = @connection.check_constraints("trades") + assert_equal 1, check_constraints.size + + cc = check_constraints.first + assert_predicate cc, :validated? + end + end + def test_remove_check_constraint @connection.add_check_constraint :trades, "price > 0", name: "price_check" @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check"