diff --git a/lib/chrono_model/adapter.rb b/lib/chrono_model/adapter.rb index 9c840539..4e391b05 100644 --- a/lib/chrono_model/adapter.rb +++ b/lib/chrono_model/adapter.rb @@ -140,8 +140,17 @@ def on_schema(schema, recurse: :follow) # Returns true if the given name references a temporal table. # def is_chrono?(table) - on_temporal_schema { data_source_exists?(table) } && - on_history_schema { data_source_exists?(table) } + table_name = if respond_to?(:extract_schema_qualified_name) + extract_schema_qualified_name(table).last + else + # AR 5 + tn = ActiveRecord::ConnectionAdapters::PostgreSQL::Utils.extract_schema_qualified_name(table.to_s) + tn.respond_to?(:identifier) ? tn.identifier : tn + end + table_name = "\"#{table_name}\"" if table_name[0] != '"' + + on_temporal_schema { data_source_exists?(table_name) } && + on_history_schema { data_source_exists?(table_name) } end # Reads the Gem metadata from the COMMENT set on the given PostgreSQL diff --git a/lib/chrono_model/adapter/ddl.rb b/lib/chrono_model/adapter/ddl.rb index 5bd21f4c..438cde62 100644 --- a/lib/chrono_model/adapter/ddl.rb +++ b/lib/chrono_model/adapter/ddl.rb @@ -78,8 +78,9 @@ def remove_history_validity_constraint(table, options = {}) # allow setting the PK to a specific value (think migration scenario). # def chrono_create_INSERT_trigger(table, pk, current, history, fields, values) + func_name = quote_identifier_name(prefix: "chronomodel_", table: table, suffix: "_insert") execute <<-SQL.strip_heredoc - CREATE OR REPLACE FUNCTION chronomodel_#{table}_insert() RETURNS TRIGGER AS $$ + CREATE OR REPLACE FUNCTION #{func_name}() RETURNS TRIGGER AS $$ BEGIN #{insert_sequence_sql(pk, current)} INTO #{current} ( #{pk}, #{fields} ) VALUES ( NEW.#{pk}, #{values} ); @@ -95,7 +96,7 @@ def chrono_create_INSERT_trigger(table, pk, current, history, fields, values) DROP TRIGGER IF EXISTS chronomodel_insert ON #{table}; CREATE TRIGGER chronomodel_insert INSTEAD OF INSERT ON #{table} - FOR EACH ROW EXECUTE PROCEDURE chronomodel_#{table}_insert(); + FOR EACH ROW EXECUTE PROCEDURE #{func_name}(); SQL end @@ -130,8 +131,9 @@ def chrono_create_UPDATE_trigger(table, pk, current, history, fields, values, op journal &= columns + func_name = quote_identifier_name(prefix: "chronomodel_", table: table, suffix: "_update") execute <<-SQL.strip_heredoc - CREATE OR REPLACE FUNCTION chronomodel_#{table}_update() RETURNS TRIGGER AS $$ + CREATE OR REPLACE FUNCTION #{func_name}() RETURNS TRIGGER AS $$ DECLARE _now timestamp; DECLARE _hid integer; DECLARE _old record; @@ -174,7 +176,7 @@ def chrono_create_UPDATE_trigger(table, pk, current, history, fields, values, op DROP TRIGGER IF EXISTS chronomodel_update ON #{table}; CREATE TRIGGER chronomodel_update INSTEAD OF UPDATE ON #{table} - FOR EACH ROW EXECUTE PROCEDURE chronomodel_#{table}_update(); + FOR EACH ROW EXECUTE PROCEDURE #{func_name}(); SQL end @@ -184,8 +186,9 @@ def chrono_create_UPDATE_trigger(table, pk, current, history, fields, values, op # DELETEd in the same transaction. # def chrono_create_DELETE_trigger(table, pk, current, history) + func_name = quote_identifier_name(prefix: "chronomodel_", table: table, suffix: "_delete") execute <<-SQL.strip_heredoc - CREATE OR REPLACE FUNCTION chronomodel_#{table}_delete() RETURNS TRIGGER AS $$ + CREATE OR REPLACE FUNCTION #{func_name}() RETURNS TRIGGER AS $$ DECLARE _now timestamp; BEGIN _now := timezone('UTC', now()); @@ -207,13 +210,14 @@ def chrono_create_DELETE_trigger(table, pk, current, history) DROP TRIGGER IF EXISTS chronomodel_delete ON #{table}; CREATE TRIGGER chronomodel_delete INSTEAD OF DELETE ON #{table} - FOR EACH ROW EXECUTE PROCEDURE chronomodel_#{table}_delete(); + FOR EACH ROW EXECUTE PROCEDURE #{func_name}(); SQL end def chrono_drop_trigger_functions_for(table_name) %w( insert update delete ).each do |func| - execute "DROP FUNCTION IF EXISTS chronomodel_#{table_name}_#{func}()" + func_name = quote_identifier_name(prefix: "chronomodel_", table: table_name, suffix: "_#{func}") + execute "DROP FUNCTION IF EXISTS #{func_name}()" end end diff --git a/lib/chrono_model/adapter/indexes.rb b/lib/chrono_model/adapter/indexes.rb index 26fb8858..8fac5408 100644 --- a/lib/chrono_model/adapter/indexes.rb +++ b/lib/chrono_model/adapter/indexes.rb @@ -70,7 +70,7 @@ def remove_timeline_consistency_constraint(table, options = {}) end def timeline_consistency_constraint_name(table) - "#{table}_timeline_consistency" + quote_identifier_name(table: table, suffix: "_timeline_consistency") end private @@ -79,9 +79,15 @@ def timeline_consistency_constraint_name(table) def chrono_create_history_indexes_for(table, p_pkey) add_temporal_indexes table, :validity, on_current_schema: true - execute "CREATE INDEX #{table}_inherit_pkey ON #{table} ( #{p_pkey} )" - execute "CREATE INDEX #{table}_recorded_at ON #{table} ( recorded_at )" - execute "CREATE INDEX #{table}_instance_history ON #{table} ( #{p_pkey}, recorded_at )" + { + "inherit_pkey" => p_pkey, + "recorded_at" => "recorded_at", + "instance_history" => "#{p_pkey}, recorded_at" + }.each do |suffix, columns| + index_name = quote_identifier_name(table: table, suffix: "_#{suffix}") + + execute "CREATE INDEX #{index_name} ON #{table} ( #{columns} )" + end end # Rename indexes on history schema @@ -153,7 +159,8 @@ def chrono_copy_indexes_to_history(table_name) # given range definition. # def temporal_index_names(table, range, options = {}) - prefix = options[:name].presence || "index_#{table}_temporal" + prefix = options[:name].presence || + quote_identifier_name(prefix: "index_", table: table, suffix: "_temporal") # When creating computed indexes # @@ -163,7 +170,7 @@ def temporal_index_names(table, range, options = {}) range = range.to_s.sub(/\W.*/, '') [range, "lower_#{range}", "upper_#{range}"].map do |suffix| - [prefix, 'on', suffix].join('_') + quote_identifier_name(table: prefix, suffix: "_on_#{suffix}") end end @@ -192,6 +199,14 @@ def chrono_alter_constraint(table_name, options) end end + def quote_identifier_name(prefix: "", table: "", suffix: "") + if table[0] == '"' && table[-1] == '"' + "\"#{prefix}#{table[1..-2]}#{suffix}\"" + else + "#{prefix}#{table}#{suffix}" + end + end + end diff --git a/spec/chrono_model/adapter/base_spec.rb b/spec/chrono_model/adapter/base_spec.rb index f0eaaf85..8d3309e1 100644 --- a/spec/chrono_model/adapter/base_spec.rb +++ b/spec/chrono_model/adapter/base_spec.rb @@ -134,6 +134,25 @@ it { expect(adapter.is_chrono?(table)).to be(false) } end + context "when passing a fully specified schema" do + before(:all) do + adapter.execute "BEGIN" + adapter.execute "CREATE SCHEMA test_schema" + + table "test_schema.#{table}" + adapter.execute "CREATE TABLE #{table} (id integer)" + end + + after(:all) do + table "test_table" + adapter.execute "ROLLBACK" + end + + it { expect { adapter.is_chrono?(table) }.to_not raise_error } + + it { expect(adapter.is_chrono?(table)).to be(false) } + end + context 'when schemas are not there yet' do before(:all) do adapter.execute 'BEGIN' @@ -150,5 +169,71 @@ it { expect(adapter.is_chrono?(table)).to be(false) } end + + context "within a different search_path" do + before(:all) do + schema_name = "schema_#{Random.rand(1000..10000)}" + + adapter.execute "BEGIN" + adapter.execute "CREATE SCHEMA #{schema_name}" + adapter.execute "SET search_path TO #{schema_name}" + end + + after(:all) do + adapter.execute "ROLLBACK" + end + + with_temporal_table do + it { expect(adapter.is_chrono?(table)).to be(true) } + end + + with_plain_table do + it { expect(adapter.is_chrono?(table)).to be(false) } + end + end + + context "with a table in a different schema" do + before(:all) do + schema_name = "schema_#{Random.rand(1000..10000)}" + + adapter.execute "BEGIN" + adapter.execute "CREATE SCHEMA #{schema_name}" + adapter.execute "SET search_path TO #{schema_name}" + + table "different_schema_table" + end + + after(:all) do + table "test_table" + + adapter.execute "ROLLBACK" + end + + with_temporal_table do + it { expect(adapter.is_chrono?(table)).to be(true) } + end + + with_plain_table do + it { expect(adapter.is_chrono?(table)).to be(false) } + end + end + + context "when the table has a period in the name" do + before(:all) do + table '"test.table"' + end + + after(:all) do + table "test_table" + end + + with_temporal_table do + it { expect(adapter.is_chrono?(table)).to be(true) } + end + + with_plain_table do + it { expect(adapter.is_chrono?(table)).to be(false) } + end + end end end