betesh / active_record-postgres-constraints Goto Github PK

As discussed in #28 (comment), to be more consistent with how rails translates nil into NULL (i.e. MyModel.where(title: [nil, 'Mr.', 'Mrs.', 'Dr.']), we should really make it possible to use t.check_constraint title: [nil, 'Mr.', 'Mrs.', 'Dr.']

The following migration:

enable_extension 'btree_gist'
    
create_table :seat_allocations, id: :uuid do |t|
  t.references :seat, type: :uuid, index: true, null: false, foreign_key: true, on_delete: :cascade
  t.datetime :from, null: false
  t.datetime :to, null: false
  t.timestamps
end
    
add_exclude_constraint :seat_allocations, using: :gist, 'tsrange("from", "to")' => :overlaps, 'cast("seat_id" AS text)' => :equals

Results in the following in the schema.rb:

# Could not dump table "seat_allocations" because of following NoMethodError
#   undefined method `include?' for nil:NilClass

Using Postgres 9.6 and Rails 5.2

This is using this gem directly from the master:

gem 'active_record-postgres-constraints', github: 'on-site/active_record-postgres-constraints'

It seems the methods of this Gem like add_check_constraint accept a constraint name as the second argument, if specified. If the name is specified explicitly, rollback seems to be a doddle (at least in Rails 6.0)! I think it is worth mentioning in README. An example of a migration file follows. Anyway, thank you for the great Gem!

class AddCheckConstraintsToWorkers < ActiveRecord::Migration[6.0]
  def change
    add_check_constraint :workers, 'check_workers_on_birth_month', "birth_month IS NULL OR birth_month BETWEEN 1 AND 12"
  end
end

Thanks for your work on this library!

We see this issue for Rails 6.1:

NameError: uninitialized constant ActiveRecord::ConnectionAdapters::AbstractAdapter::SchemaCreation
Did you mean?  ActiveRecord::ConnectionAdapters::SchemaCreation
               ActiveRecord::SchemaMigration
/home/runner/src/vendor/bundle/ruby/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/active_support.rb:80:in `block in load_missing_constant'
/home/runner/src/vendor/bundle/ruby/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/active_support.rb:9:in `without_bootsnap_cache'
/home/runner/src/vendor/bundle/ruby/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/active_support.rb:80:in `rescue in load_missing_constant'
/home/runner/src/vendor/bundle/ruby/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/active_support.rb:59:in `load_missing_constant'
/home/runner/src/vendor/bundle/ruby/2.7.0/gems/active_record-postgres-constraints-0.2.2/lib/active_record/postgres/constraints/railtie.rb:23:in `apply_patch!'
/home/runner/src/vendor/bundle/ruby/2.7.0/gems/active_record-postgres-constraints-0.2.2/lib/active_record/postgres/constraints/railtie.rb:13:in `block (2 levels) in <class:Railtie>'

Everything works fine with Rails 6.0.

Hi there, @betesh!
Thanks for the gem. It's exactly what we need. Do you mind pushing version 0.2.0 to rubygems.org? We'd love to pull the new version from there, as it contains fixes that are relevant to us.

Using rails, with rails db:schema:dump on Postgresql 12.1 my schema.rb file is full lines like the following:

# Could not dump table "add_ons" because of following ActiveRecord::StatementInvalid
#   PG::UndefinedColumn: ERROR:  column "consrc" does not exist
LINE 1: SELECT conname, consrc FROM pg_constraint JOIN pg_class ON p...
                        ^
HINT:  Perhaps you meant to reference the column "pg_constraint.conkey" or the column "pg_constraint.conbin".
: SELECT conname, consrc FROM pg_constraint JOIN pg_class ON pg_constraint.conrelid = pg_class.oid WHERE pg_constraint.contype = 'c' AND pg_class.relname = 'add_ons'

Hey! I love the idea of this gem and would love to use it in our app at work. After playing around with it for a bit, I noticed that it doesn't allow you to place custom constraints on a table. It seems that when using check_constraint, you can only specify that a column must be one of a number of values. However, Postgres already provides this capability in the form of the ENUM type. Constraints are designed to be more powerful than this, as you can use any expression you want to check any column you want.

So, I have three questions:

1. What was the use case you were trying to solve in making this gem?
2. What do you think about making it possible to add custom constraints?
3. If I were to submit a PR, how should I extend your gem to do this?

Migration ran successfully, but schema file was updated to contain something like:

ActiveRecord::Schema.define(version: 2020_03_10_101402) do

  # These are extensions that must be enabled in order to support this database
  enable_extension "citext"
  enable_extension "pgcrypto"
  enable_extension "plpgsql"

# Could not dump table "active_storage_attachments" because of following ActiveRecord::StatementInvalid
#   PG::UndefinedColumn: ERROR:  column "consrc" does not exist
LINE 1: SELECT conname, consrc, contype, pg_get_constraintdef(pg_con...
                        ^
HINT:  Perhaps you meant to reference the column "pg_constraint.conkey" or the column "pg_constraint.conbin".

# Could not dump table "active_storage_blobs" because of following ActiveRecord::StatementInvalid
#   PG::UndefinedColumn: ERROR:  column "consrc" does not exist
LINE 1: SELECT conname, consrc, contype, pg_get_constraintdef(pg_con...
                        ^
HINT:  Perhaps you meant to reference the column "pg_constraint.conkey" or the column "pg_constraint.conbin".

...

And same line continued for all tables.

Only way to fix this was to remove the gem from Gemfile. Because of this error, i am not able to use this gem for now.

t.check_constraint only works in create_table. It would be nice to be able to use it in change_table block.
For example:

change_table :people do |t|
  t.check_constraint title: ['Mr.', 'Mrs.', 'Dr.'] # add constraint
end

The line below means that if the same migration is run on multiple machines/databases, then each will get a constraint with a different name:
https://github.com/on-site/active_record-postgres-constraints/blob/beaebe651f4a46facf71812d56897ad5ac05812c/lib/active_record/postgres/constraints.rb#L40

I like the way that Rails creates a descriptive and consistent name for indexes automatically (code is here), and think it would be an improvement to copy this behaviour here. Something like constrain_foos_on_bar_and_baz.

If any of the following are true:

1. The conditions were specified in SQL so we can't easily tell what columns are used; or
2. The name would be too long; or
3. There is already a constraint with the same name;

then an error can be raised and the developer instructed to specify a name for the condition manually (also consistent with Rails behaviour).

I think this would be a good improvement, and reduce the potential for confusion caused by inconsistent constraint names. Do you agree with my thoughts here?

Following the directions in the readme results in Sprockets complaining about the lack of
spec/dummy/app/assets/config/manifest.js.

This can be corrected by running

mkdir -p spec/dummy/app/assets/config
touch spec/dummy/app/assets/config/manifest.js

I can see a few solutions:

• Check this empty file into the repo
• Add it to .gitignore and update the readme.

Hello,
I am upgrading an application from Rails 5.0 to 5.2 and I ran into an issue with database creation that seems to be caused by this gem.

When I run rake db:create RAILS_ENV=test, I get a couple related errors:

ActiveRecord::NoDatabaseError: FATAL:  database "galore_test" does not exist
PG::ConnectionBad: FATAL:  database "galore_test" does not exist

Using the --trace option, I tracked it down to this line in my application initialization:

<user>/gems/active_record-postgres-constraints-0.1.2/lib/active_record/postgres/constraints/railtie.rb:10:in `block in <class:Railtie>'

You can see that line is creating an ActiveRecord database connection. I'm not sure what changed in Rails or Railties that would cause this issue, but wanted to bring it up here.

I think we can solve this by simply rescuing the ActiveRecord::NoDatabaseError and adding a warning to the console. The entirety of that file becomes:

        initializer 'active_record.postgres.constraints.patch_active_record' do
          ActiveSupport.on_load(:active_record) do
            AR_CAS = ::ActiveRecord::ConnectionAdapters
            begin
              connection = ActiveRecord::Base.connection
              using_pg = connection.class.to_s == "#{AR_CAS}::PostgreSQLAdapter"
              if using_pg
                Rails.logger.info do
                  'Applying Postgres Constraints patches to ActiveRecord'
                end
                AR_CAS::TableDefinition.include TableDefinition
                AR_CAS::PostgreSQLAdapter.include PostgreSQLAdapter
                AR_CAS::AbstractAdapter::SchemaCreation.prepend SchemaCreation

                ::ActiveRecord::Migration::CommandRecorder.include CommandRecorder
                ::ActiveRecord::SchemaDumper.prepend SchemaDumper
              else
                Rails.logger.warn do
                  'Not applying Postgres Constraints patches to ActiveRecord ' \
                'since the database is not postgres'
                end
              end
            rescue ActiveRecord::NoDatabaseError
              Rails.logger.warn do
                'Not applying Postgres Constraints patches to ActiveRecord ' \
                'because the database does not exist'
              end
            end
          end
        end

If you'd like I can create a PR for this.

Since Rails is now in beta2, it seems like a good time to add support. In the best case scenario, this involves just loosening the dependency. 😬

Thanks for writing this awesome gem!

Just checking, is there any plans for adding unique constraints?