feat: pglogical ddl migrator

[Ziinc]

Adds a pglogical ddl migrator that wraps the migration functions and explicitly propagates the ddl statements.

Added a demo script to verify that it is working, tested on local dev.

closes O11Y-2491

pre-merge checklist:

• tidy code/docs/scripts to be more concise.

[amokan]

🤖 AI-generated review feedback (Claude Code), reviewed by @amokan

Three blocking findings, posted as inline comments: raw execute/1 SQL being wrapped in replicate_ddl_command, rollback/2 not routed through Logflare.Repo.Migrator, and no CI job exercising priv/repo/migrations/* through the wrapper (which is what keeps the first one green).

[djwhitt]

@claude Please review this.

[claude]

Claude finished @djwhitt's task in 2m 11s —— View job

---

Review of pglogical DDL migrator

• Gather context (PR description, prior review comments, changed files)
• Read core new modules (pglogical_postgres.ex, connection.ex, migrator.ex, pglogical.ex, release.ex, utils/postgres.ex)
• Check status of previously raised blocking issues (raw execute/1 wrapping, rollback/2 routing, CI coverage)
• Review migrations changes and tests
• Post final review

Note: I don't have mix/Elixir available in this environment, so I couldn't run mix compile, mix test, or mix credo — this review is from reading the code and diff only. Please run mix ci locally/in CI before merging.

Status of the three previously-flagged blocking issues (all appear resolved)

1. Raw execute/1 SQL no longer unconditionally wrapped. lib/logflare/ecto/adapters/pglogical_postgres/connection.ex:47-57 now only wraps binary DDL when Migrator.replicate_execute?() is true, defaulting to PG.execute_ddl/1 otherwise. Opt-in is via Migrator.with_replicated_execute/1 (lib/logflare/repo/migrator.ex:55-70), used deliberately per-migration (e.g. priv/repo/migrations/20260216120000_add_oban_jobs_table.exs:9, 20190220150309_delete_sources_on_account_delete.exs:7). Good catch on 20260610224521_migrate_endpoint_query_versions_item_type.exs: only the DROP INDEX is wrapped in with_replicated_execute, while the UPDATE versions ... DML is left unwrapped — avoiding the double-apply risk the earlier review flagged.
2. rollback/2 now routed through Migrator.migration_repo_for/1 (lib/logflare/release.ex:19-24), and mix ecto.rollback is wired up too via mix.exs's new rollback/1 alias — addressing both the dev/prod paths.
3. CI now migrates a real primary from zero through the wrapper: .github/workflows/pglogical-replica-ci.yml:102-110 runs mix ecto.migrate -r Logflare.Repo.Pglogical --quiet against the pglogical primary with LOGFLARE_PGLOGICAL_REPLICATE_DDL_COMMANDS_SETS set, which would have caught the ALTER SYSTEM/ALTER PUBLICATION transaction-block failures called out previously.

Tests in test/logflare/repo/migrator_test.exs and test/logflare/ecto/adapters/pglogical_postgres/connection_test.exs cover the opt-in/opt-out branches directly (wrapped vs. unwrapped, with/without replication sets, nesting, and error-path restoration of the process flag).

Remaining observations (non-blocking)

• CI path filter gap: .github/workflows/pglogical-replica-ci.yml's pull_request.paths list (lines 9-21) doesn't include priv/repo/migrations/** or mix.exs. A future PR that only adds/edits a migration (the exact class of change most likely to break DDL replication) won't trigger this job on the PR itself — it'll only be caught after merging to main (the push trigger has no path filter). Consider adding priv/repo/migrations/** to the pull_request.paths list.
• f51f9d7 (Logflare.Utils.Postgres.parse_identifier_list!/1) is a solid tightening of the earlier ad-hoc regex validation for DB_SCHEMA/replication set names — nice addition of quoted-identifier support and doctested examples.
• The pre-merge checklist item ("tidy code/docs/scripts to be more concise") is still unchecked — test/support/pglogical/demo.exs (230 lines) and test/support/pglogical/bootstrap.exs (154 lines) are sizable manual-verification scripts; worth a pass to trim before merge per the PR's own checklist, but nothing incorrect in them from a read-through.

Overall: the fixes from the earlier AI/human review round look correctly and narrowly applied, with the risky raw-SQL replication behavior now opt-in and tested, and rollback paths consistently routed. Nothing blocking found on this pass.

[djwhitt]

🤖

The CI job runs the real migrations on the pglogical primary, but these tests reset both schemas before checking the subscriber. Could we wait for and assert a representative table or column from the real migration suite on the subscriber first? The toy migrations test the replication mechanism, but not the resulting application schema.