Skip to content

Commit

Permalink
Replace internal uses of unsafe_execute with raw_execute (#103)
Browse files Browse the repository at this point in the history
We've always used the migration's code wrapper around `connection.execute`
internally to our custom migration methods so that we can get query logging for
free. That made sense when we only had two levels of "safety" here:

1. `safe_*` methods with safety checks
2. `unsafe_*` methods dispatching to the original

But we've realized that there are some basic safety features that properly fit
into even the `unsafe_*` methods, because, for example, those are generally
used to indicate the DDL may not be safe for the application (say, dropping a
table), but even if a DDL operation is unsafe for the application we ought still
to avoid systems-level impact like long-held locks. In support of that we
changed the taxonomy of methods in e3c9bd6 to make `raw_*` methods the
way to always directly dispatch to the original method while `unsafe_*` can
have basic levels of safety.

In light of that change we should use the direct dispatch method aliases when
we're implementing our own DDL, because it's expect that such calls will be
appropriately wrapped internally with the right kind of safety  (e.g., locking).

Co-authored-by: Lisa Cicatello <[email protected]>
  • Loading branch information
lmcicat and Lisa Cicatello authored Jul 10, 2024
1 parent 1548086 commit 073789b
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions lib/pg_ha_migrations/safe_statements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,25 @@ def safe_create_enum_type(name, values=nil)
when nil
raise ArgumentError, "safe_create_enum_type expects a set of values; if you want an enum with no values please pass an empty array"
when []
unsafe_execute("CREATE TYPE #{PG::Connection.quote_ident(name.to_s)} AS ENUM ()")
raw_execute("CREATE TYPE #{PG::Connection.quote_ident(name.to_s)} AS ENUM ()")
else
escaped_values = values.map do |value|
"'#{PG::Connection.escape_string(value.to_s)}'"
end
unsafe_execute("CREATE TYPE #{PG::Connection.quote_ident(name.to_s)} AS ENUM (#{escaped_values.join(',')})")
raw_execute("CREATE TYPE #{PG::Connection.quote_ident(name.to_s)} AS ENUM (#{escaped_values.join(',')})")
end
end

def safe_add_enum_value(name, value)
unsafe_execute("ALTER TYPE #{PG::Connection.quote_ident(name.to_s)} ADD VALUE '#{PG::Connection.escape_string(value)}'")
raw_execute("ALTER TYPE #{PG::Connection.quote_ident(name.to_s)} ADD VALUE '#{PG::Connection.escape_string(value)}'")
end

def unsafe_rename_enum_value(name, old_value, new_value)
if ActiveRecord::Base.connection.postgresql_version < 10_00_00
raise PgHaMigrations::InvalidMigrationError, "Renaming an enum value is not supported on Postgres databases before version 10"
end

unsafe_execute("ALTER TYPE #{PG::Connection.quote_ident(name.to_s)} RENAME VALUE '#{PG::Connection.escape_string(old_value)}' TO '#{PG::Connection.escape_string(new_value)}'")
raw_execute("ALTER TYPE #{PG::Connection.quote_ident(name.to_s)} RENAME VALUE '#{PG::Connection.escape_string(old_value)}' TO '#{PG::Connection.escape_string(new_value)}'")
end

def safe_add_column(table, column, type, options = {})
Expand Down Expand Up @@ -134,13 +134,13 @@ def safe_change_column_default(table_name, column_name, default_value)

def safe_make_column_nullable(table, column)
safely_acquire_lock_for_table(table) do
unsafe_execute "ALTER TABLE #{table} ALTER COLUMN #{column} DROP NOT NULL"
raw_execute "ALTER TABLE #{table} ALTER COLUMN #{column} DROP NOT NULL"
end
end

def unsafe_make_column_not_nullable(table, column, options={}) # options arg is only present for backwards compatiblity
safely_acquire_lock_for_table(table) do
unsafe_execute "ALTER TABLE #{table} ALTER COLUMN #{column} SET NOT NULL"
raw_execute "ALTER TABLE #{table} ALTER COLUMN #{column} SET NOT NULL"
end
end

Expand Down Expand Up @@ -261,7 +261,7 @@ def safe_add_concurrent_partitioned_index(
end

def safe_set_maintenance_work_mem_gb(gigabytes)
unsafe_execute("SET maintenance_work_mem = '#{PG::Connection.escape_string(gigabytes.to_s)} GB'")
raw_execute("SET maintenance_work_mem = '#{PG::Connection.escape_string(gigabytes.to_s)} GB'")
end

def safe_add_unvalidated_check_constraint(table, expression, name:)
Expand Down

0 comments on commit 073789b

Please sign in to comment.