From 5a0908553bcb401ab8ca3b22e4cd63f8ad468dd7 Mon Sep 17 00:00:00 2001 From: John Davis Date: Mon, 9 Jun 2025 13:10:14 -0400 Subject: [PATCH 1/9] Remove dead code --- lib/galaxy/model/triggers/history_update_time_field.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/galaxy/model/triggers/history_update_time_field.py b/lib/galaxy/model/triggers/history_update_time_field.py index 076a2aa4835..414e2d86d95 100644 --- a/lib/galaxy/model/triggers/history_update_time_field.py +++ b/lib/galaxy/model/triggers/history_update_time_field.py @@ -5,14 +5,6 @@ Database trigger installation and removal from galaxy.model.triggers.utils import execute_statements -def install_timestamp_triggers(engine): - """ - Install update_time propagation triggers for history table - """ - statements = get_timestamp_install_sql(engine.name) - execute_statements(engine, statements) - - def drop_timestamp_triggers(engine): """ Remove update_time propagation triggers for history table From 37e71e90b7842ada436364aab26a6bd7b02e9452 Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 01:48:34 -0400 Subject: [PATCH 2/9] Add trigger migration --- ..._update_trigger_to_use_clock_timestamp_.py | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py new file mode 100644 index 00000000000..3a4199c441c --- /dev/null +++ b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py @@ -0,0 +1,129 @@ +"""Update postgresql trigger to use clock_timestamp function + +NOTE: This migration will not be applied on SQLIte. + +Revision ID: a91ea1d97111 +Revises: f070559879f1 +Create Date: 2025-06-09 12:21:53.427419 + +""" + +from alembic import op + +from galaxy.model.migrations.util import _is_sqlite + +# revision identifiers, used by Alembic. +revision = "a91ea1d97111" +down_revision = "f070559879f1" +branch_labels = None +depends_on = None + + +def upgrade(): + if not _is_sqlite(): + drop_functions_and_triggers() + create_functions_and_triggers("clock_timestamp()") + + +def downgrade(): + if not _is_sqlite(): + drop_functions_and_triggers() + create_functions_and_triggers("CURRENT_TIMESTAMP") + + +def drop_functions_and_triggers(): + op.execute("DROP FUNCTION IF EXISTS fn_audit_history_by_id CASCADE") + op.execute("DROP FUNCTION IF EXISTS fn_audit_history_by_history_id CASCADE") + + +def create_functions_and_triggers(timestamp): + version = op.get_bind().engine.dialect.server_version_info[0] + if version > 10: + trigger_fn = statement_trigger_fn + trigger_def = statement_trigger_def + function_keyword = "FUNCTION" + else: + trigger_fn = row_trigger_fn + trigger_def = row_trigger_def + function_keyword = "PROCEDURE" + + sql = [] + + for id_field in ["history_id", "id"]: + function_name = f"fn_audit_history_by_{id_field}" + sql.append(trigger_fn(function_name, id_field, timestamp)) + + trigger_config = { + "history_dataset_association": "history_id", + "history_dataset_collection_association": "history_id", + "history": "id", + } + for table, id_field in trigger_config.items(): + function_name = f"fn_audit_history_by_{id_field}" + for operation in ["UPDATE", "INSERT"]: + sql.append(trigger_def(table, id_field, operation, function_keyword, function_name)) + + op.execute(sql) + + +def statement_trigger_fn(function_name, id_field, timestamp): + return f""" + CREATE OR REPLACE FUNCTION {function_name}() + RETURNS TRIGGER + LANGUAGE 'plpgsql' + AS $BODY$ + BEGIN + INSERT INTO history_audit (history_id, update_time) + SELECT DISTINCT {id_field}, {timestamp} AT TIME ZONE 'UTC' + FROM new_table + WHERE {id_field} IS NOT NULL + ON CONFLICT DO NOTHING; + RETURN NULL; + END; + $BODY$ + """ + + +def row_trigger_fn(function_name, id_field, timestamp): + return f""" + CREATE OR REPLACE FUNCTION {function_name}() + RETURNS TRIGGER + LANGUAGE 'plpgsql' + AS $BODY$ + BEGIN + INSERT INTO history_audit (history_id, update_time) + VALUES (NEW.{id_field}, {timestamp} AT TIME ZONE 'UTC') + ON CONFLICT DO NOTHING; + RETURN NULL; + END; + $BODY$ + """ + + +def statement_trigger_def(table, id_field, operation, function_keyword, function_name): + trigger_name = get_trigger_name(id_field, operation) + return f""" + CREATE TRIGGER {trigger_name} + AFTER {operation} ON {table} + REFERENCING NEW TABLE AS new_table + FOR EACH STATEMENT EXECUTE {function_keyword} {function_name}(); + """ + + +def row_trigger_def(table, id_field, operation, function_keyword, function_name): + trigger_name = get_trigger_name(id_field, operation) + return f""" + CREATE TRIGGER {trigger_name} + AFTER {operation} ON {table} + FOR EACH ROW + WHEN (NEW.{id_field} IS NOT NULL) + EXECUTE {function_keyword} {function_name}(); + """ + + +def get_trigger_name(id_field, operation): + # We always use the "s" code that denotes STATEMENT-type trigger. We do not use "r" for ROW-type + # to stay consistent with preexisting instances ("r" was only used for sqlite triggers). + operation = operation.lower()[0] # INSERT -> i, UPDATE -> u + code = f"a{operation}s" # a is AFTER, s is STATEMENT + return f"trigger_history_audit_by_{id_field}_{code}" From a2c93eaed37da2b2215e06c6e7c305118fa45a8b Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 09:41:41 -0400 Subject: [PATCH 3/9] Iterate over list of sql statements --- .../a91ea1d97111_update_trigger_to_use_clock_timestamp_.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py index 3a4199c441c..8e5e5e4200c 100644 --- a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py +++ b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py @@ -63,7 +63,8 @@ def create_functions_and_triggers(timestamp): for operation in ["UPDATE", "INSERT"]: sql.append(trigger_def(table, id_field, operation, function_keyword, function_name)) - op.execute(sql) + for stmt in sql: + op.execute(stmt) def statement_trigger_fn(function_name, id_field, timestamp): From eadf1c629b041b93bb3587951d8195371425a0b8 Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 12:08:55 -0400 Subject: [PATCH 4/9] No need to drop triggers: replace functions instead --- ..._update_trigger_to_use_clock_timestamp_.py | 56 +------------------ 1 file changed, 1 insertion(+), 55 deletions(-) diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py index 8e5e5e4200c..b27f9fb9029 100644 --- a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py +++ b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py @@ -21,49 +21,24 @@ depends_on = None def upgrade(): if not _is_sqlite(): - drop_functions_and_triggers() create_functions_and_triggers("clock_timestamp()") def downgrade(): if not _is_sqlite(): - drop_functions_and_triggers() create_functions_and_triggers("CURRENT_TIMESTAMP") -def drop_functions_and_triggers(): - op.execute("DROP FUNCTION IF EXISTS fn_audit_history_by_id CASCADE") - op.execute("DROP FUNCTION IF EXISTS fn_audit_history_by_history_id CASCADE") - - def create_functions_and_triggers(timestamp): version = op.get_bind().engine.dialect.server_version_info[0] if version > 10: trigger_fn = statement_trigger_fn - trigger_def = statement_trigger_def - function_keyword = "FUNCTION" else: trigger_fn = row_trigger_fn - trigger_def = row_trigger_def - function_keyword = "PROCEDURE" - - sql = [] for id_field in ["history_id", "id"]: function_name = f"fn_audit_history_by_{id_field}" - sql.append(trigger_fn(function_name, id_field, timestamp)) - - trigger_config = { - "history_dataset_association": "history_id", - "history_dataset_collection_association": "history_id", - "history": "id", - } - for table, id_field in trigger_config.items(): - function_name = f"fn_audit_history_by_{id_field}" - for operation in ["UPDATE", "INSERT"]: - sql.append(trigger_def(table, id_field, operation, function_keyword, function_name)) - - for stmt in sql: + stmt = trigger_fn(function_name, id_field, timestamp) op.execute(stmt) @@ -99,32 +74,3 @@ def row_trigger_fn(function_name, id_field, timestamp): END; $BODY$ """ - - -def statement_trigger_def(table, id_field, operation, function_keyword, function_name): - trigger_name = get_trigger_name(id_field, operation) - return f""" - CREATE TRIGGER {trigger_name} - AFTER {operation} ON {table} - REFERENCING NEW TABLE AS new_table - FOR EACH STATEMENT EXECUTE {function_keyword} {function_name}(); - """ - - -def row_trigger_def(table, id_field, operation, function_keyword, function_name): - trigger_name = get_trigger_name(id_field, operation) - return f""" - CREATE TRIGGER {trigger_name} - AFTER {operation} ON {table} - FOR EACH ROW - WHEN (NEW.{id_field} IS NOT NULL) - EXECUTE {function_keyword} {function_name}(); - """ - - -def get_trigger_name(id_field, operation): - # We always use the "s" code that denotes STATEMENT-type trigger. We do not use "r" for ROW-type - # to stay consistent with preexisting instances ("r" was only used for sqlite triggers). - operation = operation.lower()[0] # INSERT -> i, UPDATE -> u - code = f"a{operation}s" # a is AFTER, s is STATEMENT - return f"trigger_history_audit_by_{id_field}_{code}" From 02f5c283cfb5a50611445ac1402e90ca00345cbc Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 12:47:32 -0400 Subject: [PATCH 5/9] Account for offline mode / also fixes mypy error --- .../a91ea1d97111_update_trigger_to_use_clock_timestamp_.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py index b27f9fb9029..905719763fb 100644 --- a/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py +++ b/lib/galaxy/model/migrations/alembic/versions_gxy/a91ea1d97111_update_trigger_to_use_clock_timestamp_.py @@ -30,8 +30,9 @@ def downgrade(): def create_functions_and_triggers(timestamp): - version = op.get_bind().engine.dialect.server_version_info[0] - if version > 10: + version_info = op.get_bind().engine.dialect.server_version_info + # For offline mode (version_info is None), we assume that version > 10 + if version_info and version_info[0] > 10 or not version_info: trigger_fn = statement_trigger_fn else: trigger_fn = row_trigger_fn From c512cfeb511a1148c72578e7254f16a0149cbf7e Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 14:41:58 -0400 Subject: [PATCH 6/9] Remove dead trigger code This was last used in SQLAlchemy Migrate, removed in 22.05 --- .../triggers/history_update_time_field.py | 152 ------------------ 1 file changed, 152 deletions(-) delete mode 100644 lib/galaxy/model/triggers/history_update_time_field.py diff --git a/lib/galaxy/model/triggers/history_update_time_field.py b/lib/galaxy/model/triggers/history_update_time_field.py deleted file mode 100644 index 414e2d86d95..00000000000 --- a/lib/galaxy/model/triggers/history_update_time_field.py +++ /dev/null @@ -1,152 +0,0 @@ -""" -Database trigger installation and removal -""" - -from galaxy.model.triggers.utils import execute_statements - - -def drop_timestamp_triggers(engine): - """ - Remove update_time propagation triggers for history table - """ - statements = get_timestamp_drop_sql(engine.name) - execute_statements(engine, statements) - - -def get_timestamp_install_sql(variant): - """ - Generate a list of SQL statements for installation of timestamp triggers - """ - - sql = get_timestamp_drop_sql(variant) - - if "postgres" in variant: - # PostgreSQL has a separate function definition and a trigger - # assignment. The first two statements the functions, and - # the later assign those functions to triggers on tables - - fn_name = "update_history_update_time" - sql.append(build_pg_timestamp_fn(fn_name, "history", source_key="history_id")) - sql.append(build_pg_trigger("history_dataset_association", fn_name)) - sql.append(build_pg_trigger("history_dataset_collection_association", fn_name)) - - else: - # Other database variants are more granular. Requiring separate - # statements for INSERT/UPDATE/DELETE, and the body of the trigger - # is not necessarily reusable with a function - - for operation in ["INSERT", "UPDATE", "DELETE"]: - # change hda -> update history - sql.append( - build_timestamp_trigger(operation, "history_dataset_association", "history", source_key="history_id") - ) - - # change hdca -> update history - sql.append( - build_timestamp_trigger( - operation, "history_dataset_collection_association", "history", source_key="history_id" - ) - ) - - return sql - - -def get_timestamp_drop_sql(variant): - """ - Generate a list of statements to drop the timestamp update triggers - """ - - sql = [] - - if "postgres" in variant: - sql.append("DROP FUNCTION IF EXISTS update_history_update_time() CASCADE;") - else: - for operation in ["INSERT", "UPDATE", "DELETE"]: - for when in ["BEFORE", "AFTER"]: - sql.append(build_drop_trigger(operation, "history_dataset_association", when)) - sql.append(build_drop_trigger(operation, "history_dataset_collection_association", when)) - - return sql - - -def build_pg_timestamp_fn(fn_name, target_table, source_key, target_key="id"): - """Generates a PostgreSQL history update timestamp function""" - - return f""" - CREATE OR REPLACE FUNCTION {fn_name}() - RETURNS trigger - LANGUAGE 'plpgsql' - AS $BODY$ - BEGIN - IF (TG_OP = 'DELETE') THEN - UPDATE {target_table} - SET update_time = (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') - WHERE {target_key} = OLD.{source_key}; - RETURN OLD; - ELSEIF (TG_OP = 'UPDATE') THEN - UPDATE {target_table} - SET update_time = (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') - WHERE {target_key} = NEW.{source_key} OR {target_key} = OLD.{source_key}; - RETURN NEW; - ELSIF (TG_OP = 'INSERT') THEN - UPDATE {target_table} - SET update_time = (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') - WHERE {target_key} = NEW.{source_key}; - RETURN NEW; - END IF; - END; - $BODY$; - """ - - -def build_pg_trigger(source_table, fn_name, when="AFTER"): - """Assigns a PostgreSQL trigger to indicated table, calling user-defined function""" - when_initial = when.lower()[0] - trigger_name = f"trigger_{source_table}_{when_initial}iudr" - return f""" - CREATE TRIGGER {trigger_name} - {when} INSERT OR DELETE OR UPDATE - ON {source_table} - FOR EACH ROW - EXECUTE PROCEDURE {fn_name}(); - """ - - -def build_timestamp_trigger(operation, source_table, target_table, source_key, target_key="id", when="AFTER"): - """Creates a non-PostgreSQL update_time trigger""" - - trigger_name = get_trigger_name(operation, source_table, when) - - # three different update clauses depending on update/insert/delete - clause = "" - if operation == "DELETE": - clause = f"{target_key} = OLD.{source_key}" - elif operation == "UPDATE": - clause = f"{target_key} = NEW.{source_key} OR {target_key} = OLD.{source_key}" - else: - clause = f"{target_key} = NEW.{source_key}" - - return f""" - CREATE TRIGGER {trigger_name} - {when} {operation} - ON {source_table} - FOR EACH ROW - BEGIN - UPDATE {target_table} - SET update_time = CURRENT_TIMESTAMP - WHERE {clause}; - END; - """ - - -def build_drop_trigger(operation, source_table, when="AFTER"): - """Drops a non-PostgreSQL trigger by name""" - trigger_name = get_trigger_name(operation, source_table, when) - return f"DROP TRIGGER IF EXISTS {trigger_name}" - - -def get_trigger_name(operation, source_table, when): - """Non-PostgreSQL trigger name""" - op_initial = operation.lower()[0] - when_initial = when.lower()[0] - return f"trigger_{source_table}_{when_initial}{op_initial}r" From 24951aaea38ee9606bba3706767dae4f5842b78b Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 14:45:10 -0400 Subject: [PATCH 7/9] Move function from trigger/utils into module using it --- lib/galaxy/model/triggers/update_audit_table.py | 10 +++++++++- lib/galaxy/model/triggers/utils.py | 9 --------- 2 files changed, 9 insertions(+), 10 deletions(-) delete mode 100644 lib/galaxy/model/triggers/utils.py diff --git a/lib/galaxy/model/triggers/update_audit_table.py b/lib/galaxy/model/triggers/update_audit_table.py index 5af2ab3567c..681a784ff97 100644 --- a/lib/galaxy/model/triggers/update_audit_table.py +++ b/lib/galaxy/model/triggers/update_audit_table.py @@ -1,4 +1,4 @@ -from galaxy.model.triggers.utils import execute_statements +from sqlalchemy import DDL # function name prefix fn_prefix = "fn_audit_history_by" @@ -174,3 +174,11 @@ def get_trigger_name(label, operation, when, statement=False): when_initial = when.lower()[0] rs = "s" if statement else "r" return f"trigger_{label}_{when_initial}{op_initial}{rs}" + + +def execute_statements(engine, raw_sql): + statements = raw_sql if isinstance(raw_sql, list) else [raw_sql] + with engine.begin() as connection: + for sql in statements: + cmd = DDL(sql) + connection.execute(cmd) diff --git a/lib/galaxy/model/triggers/utils.py b/lib/galaxy/model/triggers/utils.py deleted file mode 100644 index 6644d601697..00000000000 --- a/lib/galaxy/model/triggers/utils.py +++ /dev/null @@ -1,9 +0,0 @@ -from sqlalchemy import DDL - - -def execute_statements(engine, raw_sql): - statements = raw_sql if isinstance(raw_sql, list) else [raw_sql] - with engine.begin() as connection: - for sql in statements: - cmd = DDL(sql) - connection.execute(cmd) From e1e81c0b265e831d665ad822688d5fb14674a392 Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 15:06:18 -0400 Subject: [PATCH 8/9] Update trigger creation code for new db --- lib/galaxy/model/triggers/update_audit_table.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/triggers/update_audit_table.py b/lib/galaxy/model/triggers/update_audit_table.py index 681a784ff97..9aa02b60e3e 100644 --- a/lib/galaxy/model/triggers/update_audit_table.py +++ b/lib/galaxy/model/triggers/update_audit_table.py @@ -56,7 +56,7 @@ def _postgres_install(engine): AS $BODY$ BEGIN INSERT INTO history_audit (history_id, update_time) - SELECT DISTINCT {id_field}, CURRENT_TIMESTAMP AT TIME ZONE 'UTC' + SELECT DISTINCT {id_field}, clock_timestamp() AT TIME ZONE 'UTC' FROM new_table WHERE {id_field} IS NOT NULL ON CONFLICT DO NOTHING; @@ -75,7 +75,7 @@ def _postgres_install(engine): AS $BODY$ BEGIN INSERT INTO history_audit (history_id, update_time) - VALUES (NEW.{id_field}, CURRENT_TIMESTAMP AT TIME ZONE 'UTC') + VALUES (NEW.{id_field}, clock_timestamp() AT TIME ZONE 'UTC') ON CONFLICT DO NOTHING; RETURN NULL; END; From b24d8ae6dd8437985e5d640f9dd226ee95c6ef2f Mon Sep 17 00:00:00 2001 From: John Davis Date: Tue, 10 Jun 2025 15:11:50 -0400 Subject: [PATCH 9/9] Update db revision tags --- lib/galaxy/model/migrations/dbscript.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/migrations/dbscript.py b/lib/galaxy/model/migrations/dbscript.py index 0d0b4962352..04b88411e97 100644 --- a/lib/galaxy/model/migrations/dbscript.py +++ b/lib/galaxy/model/migrations/dbscript.py @@ -46,8 +46,8 @@ REVISION_TAGS = { "24.1": "04288b6a5b25", "release_24.2": "a4c3ef999ab5", "24.2": "a4c3ef999ab5", - "release_25.0": "f070559879f1", - "25.0": "f070559879f1", + "release_25.0": "a91ea1d97111", + "25.0": "a91ea1d97111", }