Skip to content

Commit

Permalink
Ensure table names are unique and preset (#37)
Browse files Browse the repository at this point in the history 
Since we are not restoring index/sequence name for now,
ensuring the table names are unique so that if we run the same
operation on the same table, there is no sequence conflict. The sequence
will get overriden on the table anyways, but won't fail and complain
when trying to create a shadow before the swap. This is temporary until
we support restoring original index and sequence names. Can be a pro feature too
  • Loading branch information
@shayonj
shayonj authored Feb 15, 2022
1 parent 4e28155 commit 6d6b9cf
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 38 deletions.
18 changes: 13 additions & 5 deletions lib/pg_online_schema_change/orchestrate.rb
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require "securerandom"

module PgOnlineSchemaChange
class Orchestrate
extend Helper
Expand All @@ -16,6 +18,13 @@ def setup!(options)
# install functions
Query.run(client.connection, FUNC_FIX_SERIAL_SEQUENCE)
Query.run(client.connection, FUNC_CREATE_TABLE_ALL)

# Set this early on to ensure their creation and cleanup (unexpected)
# happens at all times. IOW, the calls from Store.get always return
# the same value.
Store.set(:old_primary_table, "pgosc_op_table_#{client.table}")
Store.set(:audit_table, "pgosc_at_#{client.table}_#{random_string}")
Store.set(:shadow_table, "pgosc_st_#{client.table}_#{random_string}")
end

def run!(options)
Expand Down Expand Up @@ -69,7 +78,6 @@ def handle_signals!
end

def setup_audit_table!
audit_table = Store.set(:audit_table, "pgosc_audit_table_for_#{client.table}")
logger.info("Setting up audit table", { audit_table: audit_table })

sql = <<~SQL
Expand Down Expand Up @@ -120,8 +128,6 @@ def setup_trigger!
end

def setup_shadow_table!
shadow_table = Store.set(:shadow_table, "pgosc_shadow_table_for_#{client.table}")

logger.info("Setting up shadow table", { shadow_table: shadow_table })

Query.run(client.connection, "SELECT create_table_all('#{client.table}', '#{shadow_table}');")
Expand Down Expand Up @@ -185,8 +191,6 @@ def replay_and_swap!
def swap!
logger.info("Performing swap!")

old_primary_table = Store.set(:old_primary_table, "pgosc_old_primary_table_#{client.table}")

foreign_key_statements = Query.get_foreign_keys_to_refresh(client, client.table)
storage_params_reset = primary_table_storage_parameters.empty? ? "" : "ALTER TABLE #{client.table} SET (#{primary_table_storage_parameters});"

Expand Down Expand Up @@ -243,6 +247,10 @@ def drop_and_cleanup!

Query.run(client.connection, sql)
end

private def random_string
@random_string ||= SecureRandom.hex(3)
end
end
end
end
63 changes: 30 additions & 33 deletions spec/lib/orchestrate_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,10 @@
end

it "creates the audit table with columns from parent table and additional identifiers" do
expect(client.connection).to receive(:async_exec).with("BEGIN;").and_call_original
expect(client.connection).to receive(:async_exec).with("CREATE TABLE pgosc_audit_table_for_books (operation_type text, trigger_time timestamp, LIKE books);\n").and_call_original
expect(client.connection).to receive(:async_exec).with("COMMIT;").and_call_original

described_class.setup_audit_table!

RSpec::Mocks.space.reset_all
columns = PgOnlineSchemaChange::Query.table_columns(client, "pgosc_audit_table_for_books")
columns = PgOnlineSchemaChange::Query.table_columns(client, described_class.audit_table.to_s)

expect(columns).to eq([
{ "column_name" => "\"operation_type\"", "type" => "text", "column_position" => 1,
Expand Down Expand Up @@ -121,13 +117,13 @@
$$
BEGIN
IF ( TG_OP = 'INSERT') THEN
INSERT INTO "pgosc_audit_table_for_books" select 'INSERT', now(), NEW.* ;
INSERT INTO "#{described_class.audit_table}" select 'INSERT', now(), NEW.* ;
RETURN NEW;
ELSIF ( TG_OP = 'UPDATE') THEN
INSERT INTO "pgosc_audit_table_for_books" select 'UPDATE', now(), NEW.* ;
INSERT INTO "#{described_class.audit_table}" select 'UPDATE', now(), NEW.* ;
RETURN NEW;
ELSIF ( TG_OP = 'DELETE') THEN
INSERT INTO "pgosc_audit_table_for_books" select 'DELETE', now(), OLD.* ;
INSERT INTO "#{described_class.audit_table}" select 'DELETE', now(), OLD.* ;
RETURN NEW;
END IF;
END;
Expand All @@ -144,7 +140,7 @@
expect(client.connection).to receive(:async_exec).with("COMMIT;").twice.and_call_original

described_class.setup_trigger!
expect(described_class.audit_table).to eq("pgosc_audit_table_for_books")
expect(described_class.audit_table).to eq(described_class.audit_table.to_s)
end

it "closes transaction when it couldn't acquire lock" do
Expand Down Expand Up @@ -269,11 +265,6 @@
end

it "creates the shadow table matching parent table" do
expect(client.connection).to receive(:async_exec).with("BEGIN;").and_call_original
expect(client.connection).to receive(:async_exec).with("SELECT fix_serial_sequence('books', 'pgosc_shadow_table_for_books');").and_call_original
expect(client.connection).to receive(:async_exec).with("SELECT create_table_all('books', 'pgosc_shadow_table_for_books');").and_call_original
expect(client.connection).to receive(:async_exec).with("COMMIT;").and_call_original

described_class.setup_shadow_table!

RSpec::Mocks.space.reset_all
Expand All @@ -296,22 +287,22 @@
"column_position" => 7, "column_name_regular" => "last_login" },
])

columns = PgOnlineSchemaChange::Query.get_indexes_for(client, "pgosc_shadow_table_for_books")
expect(columns).to eq(["CREATE UNIQUE INDEX pgosc_shadow_table_for_books_pkey ON pgosc_shadow_table_for_books USING btree (user_id)",
"CREATE UNIQUE INDEX pgosc_shadow_table_for_books_username_key ON pgosc_shadow_table_for_books USING btree (username)",
"CREATE UNIQUE INDEX pgosc_shadow_table_for_books_email_key ON pgosc_shadow_table_for_books USING btree (email)"])
columns = PgOnlineSchemaChange::Query.get_indexes_for(client, described_class.shadow_table.to_s)
expect(columns).to eq(["CREATE UNIQUE INDEX #{described_class.shadow_table}_pkey ON #{described_class.shadow_table} USING btree (user_id)",
"CREATE UNIQUE INDEX #{described_class.shadow_table}_username_key ON #{described_class.shadow_table} USING btree (username)",
"CREATE UNIQUE INDEX #{described_class.shadow_table}_email_key ON #{described_class.shadow_table} USING btree (email)"])

foreign_keys = PgOnlineSchemaChange::Query.get_foreign_keys_for(client,
"pgosc_shadow_table_for_books")
described_class.shadow_table.to_s)
expect(foreign_keys).to eq([
{ "table_on" => "pgosc_shadow_table_for_books", "table_from" => "sellers",
"constraint_type" => "f", "constraint_name" => "pgosc_shadow_table_for_books_seller_id_fkey", "constraint_validated" => "t", "definition" => "FOREIGN KEY (seller_id) REFERENCES sellers(id)" },
{ "table_on" => described_class.shadow_table.to_s, "table_from" => "sellers",
"constraint_type" => "f", "constraint_name" => "#{described_class.shadow_table}_seller_id_fkey", "constraint_validated" => "t", "definition" => "FOREIGN KEY (seller_id) REFERENCES sellers(id)" },
])
primary_keys = PgOnlineSchemaChange::Query.get_primary_keys_for(client,
"pgosc_shadow_table_for_books")
described_class.shadow_table.to_s)
expect(primary_keys).to eq([
{ "constraint_name" => "pgosc_shadow_table_for_books_pkey", "constraint_type" => "p",
"constraint_validated" => "t", "definition" => "PRIMARY KEY (user_id)", "table_from" => "-", "table_on" => "pgosc_shadow_table_for_books" },
{ "constraint_name" => "#{described_class.shadow_table}_pkey", "constraint_type" => "p",
"constraint_validated" => "t", "definition" => "PRIMARY KEY (user_id)", "table_from" => "-", "table_on" => described_class.shadow_table.to_s },
])
end

Expand Down Expand Up @@ -344,11 +335,11 @@

it "succesfully" do
query = <<~SQL
ALTER TABLE pgosc_shadow_table_for_books SET (
ALTER TABLE #{described_class.shadow_table} SET (
autovacuum_enabled = false, toast.autovacuum_enabled = false
);
ALTER TABLE pgosc_audit_table_for_books SET (
ALTER TABLE #{described_class.audit_table} SET (
autovacuum_enabled = false, toast.autovacuum_enabled = false
);
SQL
Expand Down Expand Up @@ -524,7 +515,7 @@

it "succesfully" do
expect(client.connection).to receive(:async_exec).with("BEGIN;").and_call_original
expect(client.connection).to receive(:async_exec).with("ALTER TABLE pgosc_shadow_table_for_books ADD COLUMN purchased boolean DEFAULT false").and_call_original
expect(client.connection).to receive(:async_exec).with("ALTER TABLE #{described_class.shadow_table} ADD COLUMN purchased boolean DEFAULT false").and_call_original
expect(client.connection).to receive(:async_exec).with("COMMIT;").and_call_original

described_class.run_alter_statement!
Expand Down Expand Up @@ -576,7 +567,7 @@

it "succesfully" do
expect(client.connection).to receive(:async_exec).with("BEGIN;").and_call_original
expect(client.connection).to receive(:async_exec).with("ALTER TABLE pgosc_shadow_table_for_books DROP email").and_call_original
expect(client.connection).to receive(:async_exec).with("ALTER TABLE #{described_class.shadow_table} DROP email").and_call_original
expect(client.connection).to receive(:async_exec).with("COMMIT;").and_call_original

described_class.run_alter_statement!
Expand Down Expand Up @@ -623,7 +614,7 @@

it "succesfully" do
expect(client.connection).to receive(:async_exec).with("BEGIN;").and_call_original
expect(client.connection).to receive(:async_exec).with("ALTER TABLE pgosc_shadow_table_for_books RENAME COLUMN email TO new_email").and_call_original
expect(client.connection).to receive(:async_exec).with("ALTER TABLE #{described_class.shadow_table} RENAME COLUMN email TO new_email").and_call_original
expect(client.connection).to receive(:async_exec).with("COMMIT;").and_call_original

described_class.run_alter_statement!
Expand Down Expand Up @@ -680,12 +671,18 @@
PgOnlineSchemaChange::Replay.play!(rows)
end

it "ensures helper table names are of proper length" do
expect(described_class.shadow_table.length).to eq(21)
expect(described_class.audit_table.length).to eq(21)
expect(described_class.old_primary_table.length).to eq(20)
end

it "sucessfully renames the tables" do
described_class.swap!

# Fetch rows from the original primary table
select_query = <<~SQL
SELECT * FROM pgosc_old_primary_table_books;
SELECT * FROM #{described_class.old_primary_table};
SQL
rows = []
PgOnlineSchemaChange::Query.run(client.connection, select_query) do |result|
Expand All @@ -705,9 +702,9 @@

# confirm indexes on newly renamed table
columns = PgOnlineSchemaChange::Query.get_indexes_for(client, "books")
expect(columns).to eq(["CREATE UNIQUE INDEX pgosc_shadow_table_for_books_pkey ON books USING btree (user_id)",
"CREATE UNIQUE INDEX pgosc_shadow_table_for_books_username_key ON books USING btree (username)",
"CREATE UNIQUE INDEX pgosc_shadow_table_for_books_email_key ON books USING btree (email)"])
expect(columns).to eq(["CREATE UNIQUE INDEX #{described_class.shadow_table}_pkey ON books USING btree (user_id)",
"CREATE UNIQUE INDEX #{described_class.shadow_table}_username_key ON books USING btree (username)",
"CREATE UNIQUE INDEX #{described_class.shadow_table}_email_key ON books USING btree (email)"])
end

it "sucessfully drops the trigger" do
Expand Down

0 comments on commit 6d6b9cf

Please sign in to comment.