Giter VIP home page Giter VIP logo

safe-pg-migrations's Issues

Support default value on a table where id is a UUID

Feature request

Current state

safe-pg-migrations doesn't support to add a default value on table with a UUID primary key.

If you do, you will get the following error:

PG::UndefinedFunction: ERROR:  operator does not exist: uuid > integer
LINE 1: SELECT id FROM "mytable" WHERE id > 0 ORDER BY id LIMIT 10...

This issue is related to this line.

What is the expected behavior?

That safe-pg-migrations supports both integer and uuid for primary key id.

Error undefined method `table' for "index_name":String on v1.4.0

Hi, thanks for creating an amazing gem.

I ran into an issue on the recent 1.4.0 update, getting an error undefined method 'table' for "index_name":String when running a create table migration with reference or belongs_to
I tested It on 1.3.0 and it works fine

My env:
ruby 2.7.4
rails 5.2.5

Sample migration

class CreateComments < ActiveRecord::Migration[5.2]
  def change
    create_table :comments do |t|
      t.references :user, foreign_key: true
      t.string :text, null: false

      t.timestamps
    end
  end
end

Logs:

Caused by:
NoMethodError: undefined method `table' for "index_comments_on_user_id":String
Did you mean?  tableize
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/idempotent_statements.rb:10:in `add_index'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:76:in `block in add_index'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:121:in `block (2 levels) in without_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:117:in `block in without_lock_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:100:in `with_setting'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:117:in `without_lock_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:121:in `block in without_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:113:in `block in without_statement_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:100:in `with_setting'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:113:in `without_statement_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:121:in `without_timeout'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:76:in `add_index'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/useless_statements_logger.rb:15:in `add_index'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/connection_adapters/abstract/schema_statements.rb:315:in `block in create_table'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/connection_adapters/abstract/schema_statements.rb:314:in `each'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/connection_adapters/abstract/schema_statements.rb:314:in `create_table'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:22:in `block (3 levels) in <module:BlockingActivityLogger>'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:65:in `log_blocking_queries'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:22:in `block (2 levels) in <module:BlockingActivityLogger>'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/idempotent_statements.rb:58:in `create_table'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:57:in `block in create_table'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:100:in `with_setting'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:56:in `create_table'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:871:in `block in method_missing'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:840:in `block in say_with_time'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:840:in `say_with_time'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:860:in `method_missing'
/Users/hassan/moodpath-rails/db/migrate/20220227212107_create_comments.rb:3:in `change'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:814:in `exec_migration'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/base.rb:81:in `block in exec_migration'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/base.rb:34:in `block in setup_and_teardown'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/plugins/statement_insurer.rb:100:in `with_setting'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/base.rb:34:in `setup_and_teardown'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/safe-pg-migrations-1.4.0/lib/safe-pg-migrations/base.rb:80:in `exec_migration'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:798:in `block (2 levels) in migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:797:in `block in migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/connection_adapters/abstract/connection_pool.rb:416:in `with_connection'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:796:in `migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:977:in `migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1292:in `block in execute_migration_in_transaction'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1345:in `ddl_transaction'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1291:in `execute_migration_in_transaction'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1263:in `block in migrate_without_lock'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1262:in `each'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1262:in `migrate_without_lock'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1210:in `block in migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1363:in `with_advisory_lock'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1210:in `migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1036:in `up'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/migration.rb:1011:in `migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/tasks/database_tasks.rb:172:in `migrate'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activerecord-5.2.5/lib/active_record/railties/databases.rake:60:in `block (2 levels) in <top (required)>'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activesupport-5.2.5/lib/active_support/dependencies.rb:285:in `load'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activesupport-5.2.5/lib/active_support/dependencies.rb:285:in `block in load'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activesupport-5.2.5/lib/active_support/dependencies.rb:257:in `load_dependency'
/Users/hassan/.rvm/gems/ruby-2.7.4@moodpath-rails/gems/activesupport-5.2.5/lib/active_support/dependencies.rb:285:in `load'

Escape hatch for unsafe migrations

Hi!

Some migrations unfortunately cannot be run safely and are then impossible to run.
When using a partitioned table on PostgreSQL, this migration, where slots is the partitioned table, will fail because of 2 Postgres errors:

    add_reference :slots, :variation, index: true, foreign_key: true
  1. ActiveRecord::StatementInvalid: PG::WrongObjectType: ERROR: cannot add NOT VALID foreign key on partitioned table "slots" referencing relation "variations": since validate: false is forced
  2. PG::FeatureNotSupported: ERROR: cannot create index on partitioned table "stock_slots" concurrently : since algorithm: :concurrently is forced

Is there a way to explicitly disable the safeness in a block? If not, what about a block:

  unsafe_migration do
    add_reference :slots, :variation, index: true, foreign_key: true, validate: true
  end

Telling: I know it is not safe, and then it can be caught before deployment to take appropriate measures.

Adding a foreign key causes `ArgumentError: unknown keyword: :column`

Hello 👋

First of all thank you for sharing this very handy gem which works out of the box and solves such a great pain 🙌

Problem
I ran into an issue using the latest 1.3.0 version of the gem on a ruby 2.7.4 rails 5.2.6 project.
Adding a foreign key to a table with add_reference triggered an ArgumentError: unknown keyword: :column when running rails db:migrate. I reproduced the error on a vanilla rails new project with the same versions to let you reproduce the 🐛 easily 👌
Here is the backtrace 👇

== 20211015145629 AddRestaurantReferenceToReviews: migrating ==================
-- add_reference(:reviews, :restaurant, {:foreign_key=>true})
   -> add_index("reviews", ["restaurant_id"], {:algorithm=>:concurrently})
rails aborted!
StandardError: An error has occurred, all later migrations canceled:

unknown keyword: :column
/Users/edwardschults/code/Eschults/safe-pg-add-reference/db/migrate/20211015145629_add_restaurant_reference_to_reviews.rb:3:in `change'
/Users/edwardschults/code/Eschults/safe-pg-add-reference/bin/rails:9:in `<top (required)>'
/Users/edwardschults/code/Eschults/safe-pg-add-reference/bin/spring:15:in `<top (required)>'
./bin/rails:3:in `load'
./bin/rails:3:in `<main>'

Caused by:
ArgumentError: unknown keyword: :column
/Users/edwardschults/code/Eschults/safe-pg-add-reference/db/migrate/20211015145629_add_restaurant_reference_to_reviews.rb:3:in `change'
/Users/edwardschults/code/Eschults/safe-pg-add-reference/bin/rails:9:in `<top (required)>'
/Users/edwardschults/code/Eschults/safe-pg-add-reference/bin/spring:15:in `<top (required)>'
./bin/rails:3:in `load'
./bin/rails:3:in `<main>'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

After debugging and step-ping in the source code, I found out that the error was coming from this line, which changed between 1.2.3 and 1.3.0 with the addition of support for Ruby 3.0.

Note that when this error occurs, the creation of the column actually goes through, but not the creation of the schema_migrations record, making any new rails db:migrate run into the error again and preventing next migrations to run.

Solution
Downgrading to 1.2.3 solves the error on ruby 2.7 👌

Cheers!

Option to disable IdempotentStatements?

Would you be open to adding an option to disable IdempotentStatements?

Idempotency is great, but the current code doesn't go quite far enough currently to ensure actual idempotence imo. E.g. it only verifies that a column name exists, not that it matches the column being requested. I know I've made mistakes along those lines in the past, and I'd rather get an error message instead of having it be ignored.

Are you using the idempotency approach in general to lessen the effect of disabling transactional migrations? Are there other aspects I'm not thinking about?

question: Is there a way to disable logging for some migrations?

We chose to go with this gem over strong-migrations because of the nicer logging. However if does not play well with a Gem called data-migrate. In the sense that the logging is really verbose, where updating thousands of records will create tens of thousands of migration logs.

So is there a way to disable the logging for some migrations but not others perhaps a class method or option ?

`SET` and `disable_ddl_transaction!`

Those two don't play too well together when used via PgBouncer in transactional pooling mode.
Transactional pooling mode is the default for server-side (Postgres-side) PgBouncer installation on Heroku.

SET statement_timeout/CREATE INDEX CONCURRENTLY and reset of statement_timeout could run in three different server connections. Because of disable_ddl_transaction!. There are two problems here:

  • unsettling statement_timeout has no effect on CREATE INDEX CONCURRENTLY's timeout if they are executed in different server connections
  • we leave a connection hanging in PgBouncer's pool with unset statement_timeout

It makes sense to mention that somewhere in the documentation.

I can handle sending a PR if you confirm and approve the problem.

Wrong number of arguments - Rails 6.1

When using with Rails 6.1 I receive a lot of times error:

StandardError: An error has occurred, all later migrations canceled:

wrong number of arguments (given 4, expected 3)

Migration:

add_column :actions, :approval_level_id, :bigint, index: true

Is this gem compatible with latest Rails release?

add_column, null: false should be executed in one statement

Such a migration

class AddAwesomeNotesToPatient
    def change
        add_column :awesome_notes, :string, array: true, default: [],  null: false
    end
end

is using two alter table statements whereas it should only use one.

image

The ideal statement should be

ALTER TABLE "patients" ADD "awesome_notes" character varying[] DEFAULT '{}' NOT NULL

Idea: only disable DDL transaction for transactions that use helpers

That is practically achievable by providing a SafeMigration concern that users could include in their migrations.
It will:

  • provide updated helpers
  • set lock and statement timeouts
  • disable DDL transaction

This approach would still allow having migrations that have DDL transaction turned on.

blocking_activity_logger: can't convert String into an exact number

safe-pg-migrations/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb

Lines 68 to 87 in 07a3551

rescue ActiveRecord::LockWaitTimeout
SafePgMigrations.say 'Lock timeout.', true
queries =
begin
blocking_queries_retriever_thread.value
rescue StandardError => e
SafePgMigrations.say("Error while retrieving blocking queries: #{e}", true)
nil
end
raise if queries.nil?
if queries.empty?
SafePgMigrations.say 'Could not find any blocking query.', true
else
SafePgMigrations.say(
"Statement was being blocked by the following #{'query'.pluralize(queries.size)}:", true
)
SafePgMigrations.say '', true
output_blocking_queries(queries)

currently debugging locally a (probably gem versioning) issue where this value has a string timestamp like:

[["select pg_sleep(300), id from table limit 1;", "2021-09-27 14:58:18.677782+00"]]

and then the time formatter blows up with:

can't convert String into an exact number
--
 0: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/activesupport-5.2.6/lib/active_support/core_ext/time/calculations.rb:275:in `-'
 1: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/activesupport-5.2.6/lib/active_support/core_ext/time/calculations.rb:275:in `minus_with_duration'
 2: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/activesupport-5.2.6/lib/active_support/core_ext/time/calculations.rb:286:in `minus_with_coercion'
 3: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/safe-pg-migrations-1.2.3/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:115:in `format_start_time'
 4: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/safe-pg-migrations-1.2.3/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:100:in `block in output_blocking_queries'
 5: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/safe-pg-migrations-1.2.3/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:100:in `each'
 6: /home/me/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/safe-pg-migrations-1.2.3/lib/safe-pg-migrations/plugins/blocking_activity_logger.rb:100:in `output_blocking_queries'
 [...]

Version 2.2.1+ not allowing remove_column to contain 4 arguments

Steps to reproduce:

  • Set safe-pg-migrations to 2.2.1+ version
  • In a migration do a safety_assured and within the block call a remove_column with 4 arguments.
    e.g:
safety_assured do 
  remove_column :users, :first_name, :boolean, null: false, default: false
end

the 4th arguments is a hash containing null: false and default: false

error:

rake aborted!
StandardError: An error has occurred, all later migrations canceled:

wrong number of arguments (given 4, expected 2..3)

I've been looking at recent commits that were supplied, but wasn't able to find in which one this issue was introduced.

Can't rollback an add_index migration

This gem is a fantastic idea! Thank you for the work to make it available to the community.

I'm attempting to use it in a project, and ran into an issue straight away.

$ bundle info safe-pg-migrations
  * safe-pg-migrations (1.2.3)
	Summary: Make your PG migrations safe.
	Homepage: https://github.com/doctolib/safe-pg-migrations
	Path: /Users/ylansegal/.asdf/installs/ruby/3.0.1/lib/ruby/gems/3.0.0/gems/safe-pg-migrations-1.2.3

Here is my test migration:

class AddTestIndex < ActiveRecord::Migration[6.1]
  def change
    add_index :users, :created_at
  end
end

I can run db:migrate and see the expected safe operations for concurrently:

$ rails db:migrate
== 20210617192601 AddTestIndex: migrating =====================================
-- add_index(:users, :created_at)
   -> add_index("users", :created_at, {:algorithm=>:concurrently})
   -> 0.0087s
== 20210617192601 AddTestIndex: migrated (0.0118s) ============================

However, I encounter an error when I try to rollback:

$ rails db:rollback
== 20210617192601 AddTestIndex: reverting =====================================
-- remove_index(:users, :created_at)
   -> remove_index("users", {:column=>:created_at, :algorithm=>:concurrently})
rails aborted!
StandardError: An error has occurred, all later migrations canceled:

No indexes found on users with the options provided.

Caused by:
ArgumentError: No indexes found on users with the options provided.

Thank you!

safe rename_column seems to be missing?

Rename column is missing from the safe rails methods, is this intended?

safe-pg-migrations/lib/safe-pg-migrations/base.rb

Lines 84 to 92 in 267c2b7

SAFE_METHODS = %i[
execute
add_column
add_index
add_reference
add_belongs_to
change_column_null
add_foreign_key
].freeze

I ask because the alter table statement to rename a column will need an access exclusive lock, no?

rails also drops and recreates the index, using add_index which is a safe method.

https://github.com/rails/rails/blob/18da7b6ba71c5ae2ed212e9f3a9e8f6dd0732b12/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L462-L466

Based on all of this, but I could be missing something, I would expect this gem to protect rename_column.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.