From c8acc2091db5709bc768153c748bf3f5dcd0858c Mon Sep 17 00:00:00 2001 From: samo-agent <280144521+samo-agent@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:20:58 +0000 Subject: [PATCH 1/2] test: expose ambient installer marker bypass (#202) Exercise a marker supplied through PGOPTIONS against both the immutable v1.5 payload and an empty database. A forced late DDL failure must leave version, config data and identity, and the ash schema exactly unchanged. --- devel/tests/config_normalization_atomicity.sh | 301 +++++++++++++++++- 1 file changed, 295 insertions(+), 6 deletions(-) diff --git a/devel/tests/config_normalization_atomicity.sh b/devel/tests/config_normalization_atomicity.sh index 6a2af47..930c51f 100644 --- a/devel/tests/config_normalization_atomicity.sh +++ b/devel/tests/config_normalization_atomicity.sh @@ -11,8 +11,11 @@ IFS=$'\n\t' readonly REQUESTED_CASE="${1:-all}" if [[ "${REQUESTED_CASE}" != "all" \ && "${REQUESTED_CASE}" != "custom-trigger" \ - && "${REQUESTED_CASE}" != "dependent-view" ]]; then - printf 'usage: %s [all|custom-trigger|dependent-view]\n' "$0" >&2 + && "${REQUESTED_CASE}" != "dependent-view" \ + && "${REQUESTED_CASE}" != "external-marker" ]]; then + printf \ + 'usage: %s [all|custom-trigger|dependent-view|external-marker]\n' \ + "$0" >&2 exit 2 fi @@ -65,6 +68,8 @@ cleanup_database() { "${PSQL[@]}" --quiet >/dev/null 2>&1 <<'SQL' || true drop event trigger if exists fail_ash_install_transaction_marker; drop function if exists public.fail_ash_install_transaction_marker(); +drop event trigger if exists fail_ash_install_external_marker; +drop function if exists public.fail_ash_install_external_marker(); do $cleanup$ begin if pg_catalog.to_regprocedure('ash.uninstall(text)') is not null then @@ -104,6 +109,15 @@ reset_test_database() { " } +reset_empty_test_database() { + "${PSQL_MAINTENANCE[@]}" \ + --quiet \ + --command="drop database if exists ${TEST_DATABASE} with (force);" + "${PSQL_MAINTENANCE[@]}" \ + --quiet \ + --command="create database ${TEST_DATABASE} template template0;" +} + cleanup() { cleanup_database drop_owned_databases @@ -170,6 +184,275 @@ snapshot_schema() { --file="${REPO_ROOT}/devel/tests/schema_snapshot.sql" >"${output_file}" } +optional_config_state() { + local config_exists + config_exists="$("${PSQL[@]}" \ + --tuples-only \ + --no-align \ + --command=" + select pg_catalog.to_regclass('ash.config') is not null; + ")" + if [[ "${config_exists}" == "t" ]]; then + config_state + else + printf '||\n' + fi +} + +config_data_hash() { + local config_exists + config_exists="$("${PSQL[@]}" \ + --tuples-only \ + --no-align \ + --command=" + select pg_catalog.to_regclass('ash.config') is not null; + ")" + if [[ "${config_exists}" == "t" ]]; then + "${PSQL[@]}" \ + --tuples-only \ + --no-align \ + --command=" + select pg_catalog.md5(pg_catalog.to_jsonb(config_row)::text) + from ash.config as config_row + where singleton; + " + else + printf '\n' + fi +} + +config_relation_oid() { + "${PSQL[@]}" \ + --tuples-only \ + --no-align \ + --command=" + select coalesce( + pg_catalog.to_regclass('ash.config')::oid::text, + '' + ); + " +} + +install_external_marker_failure_trigger() { + "${PSQL[@]}" --quiet <<'SQL' +create function public.fail_ash_install_external_marker() +returns event_trigger +language plpgsql +as $event_trigger$ +declare + command record; +begin + for command in + select * + from pg_catalog.pg_event_trigger_ddl_commands() + loop + if command.object_type = 'function' + and command.schema_name = 'ash' + and command.object_identity like 'ash.summary(%' then + raise exception + 'forced late installer failure under externally preset marker'; + end if; + end loop; +end +$event_trigger$; + +create event trigger fail_ash_install_external_marker +on ddl_command_end +when tag in ('CREATE FUNCTION') +execute function public.fail_ash_install_external_marker(); +SQL +} + +assert_external_marker_v15_direct_installer_rolled_back() { + local install_log="${TEST_TMP_DIR}/external-marker-v15.install.log" + local before_snapshot="${TEST_TMP_DIR}/external-marker-v15.before.snapshot" + local after_snapshot="${TEST_TMP_DIR}/external-marker-v15.after.snapshot" + local snapshot_diff="${TEST_TMP_DIR}/external-marker-v15.snapshot.diff" + local before_state + local after_state + local before_config_hash + local after_config_hash + local before_config_oid + local after_config_oid + local install_exit + local blocker_survives + local marker_options="${PGOPTIONS:+${PGOPTIONS} }-c pg_ash.install_in_migration_transaction=on" + + reset_test_database + install_external_marker_failure_trigger + + before_state="$(config_state)" + before_config_hash="$(config_data_hash)" + before_config_oid="$(config_relation_oid)" + snapshot_schema "${before_snapshot}" + printf 'Issue #202 external-marker v1.5 precondition: %s\n' \ + "${before_state}" + if [[ "${before_state}" != "1.5|1.5|11" ]]; then + printf 'unexpected external-marker v1.5 precondition: %s\n' \ + "${before_state}" >&2 + return 1 + fi + + set +e + env PGOPTIONS="${marker_options}" \ + "${PSQL[@]}" \ + --file="${REPO_ROOT}/sql/ash-install.sql" \ + >"${install_log}" 2>&1 + install_exit=$? + set -e + + printf 'Issue #202 external-marker v1.5 install_exit=%s\n' \ + "${install_exit}" + if ((install_exit != 3)); then + sed -n '1,240p' "${install_log}" >&2 + printf 'external-marker v1.5 install: expected exit 3, got %s\n' \ + "${install_exit}" >&2 + return 1 + fi + if ! grep -F -- \ + "forced late installer failure under externally preset marker" \ + "${install_log}" >/dev/null; then + sed -n '1,240p' "${install_log}" >&2 + printf 'external-marker v1.5 forced failure was not observed\n' >&2 + return 1 + fi + + after_state="$(config_state)" + after_config_hash="$(config_data_hash)" + after_config_oid="$(config_relation_oid)" + printf 'Issue #202 external-marker v1.5 after failure: %s\n' \ + "${after_state}" + if [[ "${after_state}" != "${before_state}" ]]; then + sed -n '1,240p' "${install_log}" >&2 + printf 'external marker published a partial v1.5 upgrade: before=%s after=%s\n' \ + "${before_state}" "${after_state}" >&2 + return 1 + fi + if [[ "${after_config_hash}" != "${before_config_hash}" ]]; then + printf 'external-marker failure changed ash.config data: before=%s after=%s\n' \ + "${before_config_hash}" "${after_config_hash}" >&2 + return 1 + fi + if [[ "${after_config_oid}" != "${before_config_oid}" ]]; then + printf 'external-marker failure replaced ash.config: before=%s after=%s\n' \ + "${before_config_oid}" "${after_config_oid}" >&2 + return 1 + fi + + snapshot_schema "${after_snapshot}" + if ! diff -u \ + "${before_snapshot}" "${after_snapshot}" >"${snapshot_diff}"; then + sed -n '1,240p' "${snapshot_diff}" >&2 + printf 'external-marker v1.5 failure changed the ash schema\n' >&2 + return 1 + fi + + blocker_survives="$("${PSQL[@]}" \ + --tuples-only \ + --no-align \ + --command=" + select exists ( + select + from pg_catalog.pg_event_trigger + where evtname = 'fail_ash_install_external_marker' + ); + ")" + if [[ "${blocker_survives}" != "t" ]]; then + printf 'external-marker v1.5 failure removed its blocker\n' >&2 + return 1 + fi + + printf 'Issue #202 external-marker v1.5 rollback PASSED\n' +} + +assert_external_marker_fresh_installer_rolled_back() { + local install_log="${TEST_TMP_DIR}/external-marker-fresh.install.log" + local before_snapshot="${TEST_TMP_DIR}/external-marker-fresh.before.snapshot" + local after_snapshot="${TEST_TMP_DIR}/external-marker-fresh.after.snapshot" + local snapshot_diff="${TEST_TMP_DIR}/external-marker-fresh.snapshot.diff" + local before_state + local after_state + local before_config_hash + local after_config_hash + local install_exit + local blocker_survives + local marker_options="${PGOPTIONS:+${PGOPTIONS} }-c pg_ash.install_in_migration_transaction=on" + + reset_empty_test_database + install_external_marker_failure_trigger + + before_state="$(optional_config_state)" + before_config_hash="$(config_data_hash)" + snapshot_schema "${before_snapshot}" + if [[ "${before_state}" != "||" \ + || "${before_config_hash}" != "" ]]; then + printf 'unexpected external-marker fresh precondition: state=%s hash=%s\n' \ + "${before_state}" "${before_config_hash}" >&2 + return 1 + fi + + set +e + env PGOPTIONS="${marker_options}" \ + "${PSQL[@]}" \ + --file="${REPO_ROOT}/sql/ash-install.sql" \ + >"${install_log}" 2>&1 + install_exit=$? + set -e + + printf 'Issue #202 external-marker fresh install_exit=%s\n' \ + "${install_exit}" + if ((install_exit != 3)); then + sed -n '1,240p' "${install_log}" >&2 + printf 'external-marker fresh install: expected exit 3, got %s\n' \ + "${install_exit}" >&2 + return 1 + fi + if ! grep -F -- \ + "forced late installer failure under externally preset marker" \ + "${install_log}" >/dev/null; then + sed -n '1,240p' "${install_log}" >&2 + printf 'external-marker fresh forced failure was not observed\n' >&2 + return 1 + fi + + after_state="$(optional_config_state)" + after_config_hash="$(config_data_hash)" + printf 'Issue #202 external-marker fresh after failure: %s\n' \ + "${after_state}" + if [[ "${after_state}" != "${before_state}" \ + || "${after_config_hash}" != "${before_config_hash}" ]]; then + sed -n '1,240p' "${install_log}" >&2 + printf 'external marker left a partial fresh install: state=%s hash=%s\n' \ + "${after_state}" "${after_config_hash}" >&2 + return 1 + fi + + snapshot_schema "${after_snapshot}" + if ! diff -u \ + "${before_snapshot}" "${after_snapshot}" >"${snapshot_diff}"; then + sed -n '1,240p' "${snapshot_diff}" >&2 + printf 'external-marker fresh failure left a partial ash schema\n' >&2 + return 1 + fi + + blocker_survives="$("${PSQL[@]}" \ + --tuples-only \ + --no-align \ + --command=" + select exists ( + select + from pg_catalog.pg_event_trigger + where evtname = 'fail_ash_install_external_marker' + ); + ")" + if [[ "${blocker_survives}" != "t" ]]; then + printf 'external-marker fresh failure removed its blocker\n' >&2 + return 1 + fi + + printf 'Issue #202 external-marker fresh rollback PASSED\n' +} + assert_failed_upgrade_rolled_back() { local case_name=$1 local expected_error=$2 @@ -273,7 +556,7 @@ begin true ), '' - ) = 'on' then + ) <> '' then raise exception 'transaction-local installer marker survived rollback'; else raise exception 'forced direct installer failure after marker rollback'; @@ -297,15 +580,15 @@ SQL fi snapshot_schema "${before_snapshot}" - # In one psql process, establish and roll back exactly the transaction-local - # marker used by the migration, then invoke the installer directly. The + # In one psql process, establish and roll back the transaction-bound token + # used by the migration, then invoke the installer directly. The # event trigger fails late, after the version stamp and most DDL. If the # marker survived, the error text differs; if direct invocation lost its # owned transaction, the state/schema checks below expose partial commits. set +e printf '%s\n' \ 'begin;' \ - "set local pg_ash.install_in_migration_transaction = 'on';" \ + "select pg_catalog.set_config('pg_ash.install_in_migration_transaction', pg_catalog.pg_current_xact_id()::text, true);" \ 'rollback;' \ "\\i '${REPO_ROOT}/sql/ash-install.sql'" | "${PSQL[@]}" >"${interactive_log}" 2>&1 @@ -416,5 +699,11 @@ if [[ "${REQUESTED_CASE}" == "all" ]]; then assert_transaction_marker_does_not_leak fi +if [[ "${REQUESTED_CASE}" == "all" \ + || "${REQUESTED_CASE}" == "external-marker" ]]; then + assert_external_marker_v15_direct_installer_rolled_back + assert_external_marker_fresh_installer_rolled_back +fi + cleanup_database printf 'Issue #202 failed-normalization release-state atomicity PASSED\n' From 38af9f2f9b462027a484bd42ae0c273ecfe32a28 Mon Sep 17 00:00:00 2001 From: samo-agent <280144521+samo-agent@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:21:04 +0000 Subject: [PATCH 2/2] fix: bind installer mode to migration transaction (#202) The installer previously trusted the literal internal GUC value, so an ambient PGOPTIONS setting could suppress both transaction boundaries. Store the migration's transaction ID as the local token, accept include mode only when it matches the currently assigned transaction, and cache that decision through the footer. Direct invocations now retain their transaction while the cumulative migration still owns one atomic installer-plus-normalization transaction. --- sql/ash-install.sql | 30 +++++++++++------------------- sql/migrations/ash-1.5-to-2.0.sql | 17 +++++++++++------ 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/sql/ash-install.sql b/sql/ash-install.sql index 1bb0aaf..ad8b483 100644 --- a/sql/ash-install.sql +++ b/sql/ash-install.sql @@ -16,23 +16,27 @@ * \i inside an existing transaction, the final COMMIT also commits the * caller's outer work. * - * The current cumulative migration sets an internal transaction-local setting - * so it can include this installer inside the migration's wider transaction. - * That keeps the 2.0 stamp and the migration's physical convergence work - * atomic. The setting, unlike a psql variable, cannot leak after a failed - * migration is rolled back in an interactive session. + * The current cumulative migration sets an internal transaction-local token + * to its transaction ID so it can include this installer inside the + * migration's wider transaction. The installer accepts include mode only + * when that token matches the current assigned transaction ID; an ambient + * setting (for example, one supplied through PGOPTIONS) cannot suppress the + * installer's transaction boundaries. The decision is cached in a psql + * variable through the footer so later SQL cannot change transaction + * ownership mid-install. This keeps the 2.0 stamp and the migration's physical + * convergence work atomic. */ \set ON_ERROR_ROLLBACK off \set ON_ERROR_STOP on select case - when coalesce( + when nullif( pg_catalog.current_setting( 'pg_ash.install_in_migration_transaction', true ), '' - ) = 'on' then 'true' + ) = pg_catalog.pg_current_xact_id_if_assigned()::text then 'true' else 'false' end as pg_ash_install_in_migration_transaction \gset @@ -40,7 +44,6 @@ end as pg_ash_install_in_migration_transaction \else begin; \endif -\unset pg_ash_install_in_migration_transaction /* * Preserve function EXECUTE grants across the drop/recreate below (#107). @@ -7897,17 +7900,6 @@ begin end if; end $$; -select case - when coalesce( - pg_catalog.current_setting( - 'pg_ash.install_in_migration_transaction', - true - ), - '' - ) = 'on' then 'true' - else 'false' -end as pg_ash_install_in_migration_transaction -\gset \if :pg_ash_install_in_migration_transaction \else commit; diff --git a/sql/migrations/ash-1.5-to-2.0.sql b/sql/migrations/ash-1.5-to-2.0.sql index 08f8163..4d31eab 100644 --- a/sql/migrations/ash-1.5-to-2.0.sql +++ b/sql/migrations/ash-1.5-to-2.0.sql @@ -34,11 +34,12 @@ * exits without replacing the table once the canonical order is present. * * One migration-owned transaction covers both phases. The installer normally - * owns and commits its transaction; the transaction-local setting below makes - * it participate in this wider transaction instead. A finite preflight cannot - * prove that normalization will finish: the explicit catalog checks reject - * known unsupported table shapes, but a permitted dependent view can still - * make DROP RESTRICT fail, and event triggers, concurrent DDL, permissions, + * owns and commits its transaction; the transaction-local token below binds + * include mode to this transaction ID so the installer participates in the + * wider transaction instead. A finite preflight cannot prove that + * normalization will finish: the explicit catalog checks reject known + * unsupported table shapes, but a permitted dependent view can still make + * DROP RESTRICT fail, and event triggers, concurrent DDL, permissions, * cancellation, or resource/commit failures can arise later. Keeping every * statement in one transaction guarantees that any such failure rolls back * the 2.0 version stamp together with all installer and normalization changes. @@ -46,7 +47,11 @@ \set ON_ERROR_STOP on begin; -set local pg_ash.install_in_migration_transaction = 'on'; +select pg_catalog.set_config( + 'pg_ash.install_in_migration_transaction', + pg_catalog.pg_current_xact_id()::text, + true +); \ir ../ash-install.sql set local search_path = pg_catalog, pg_temp;