Skip to content

Apply v1.0.18 #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nettrash merged 4 commits into from
Apr 25, 2026
Merged

Apply v1.0.18 #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d61121
Fix grants for former owners after owner change (#174)
iBarBuba Apr 25, 2026
04c2d15
v1.0.18
nettrash Apr 25, 2026
bb7ce16
v1.0.18
nettrash Apr 25, 2026
7745870
v1.0.18
nettrash Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 149 additions & 1 deletion .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,157 @@ jobs:
cd ${{ github.workspace }}/app ;
cargo test --verbose

docker:
integration:
needs: build
runs-on: ubuntu-latest

# Round-trip test against every supported PostgreSQL major version.
# For each PG version the job:
# 1. Spins up a fresh postgres container (with wal_level=logical so
# the publication tests in the schemas don't fail at CREATE).
# 2. Creates two databases `a` and `b`.
# 3. Applies `data/test/schema_a.sql` to `a` and `schema_b.sql` to `b`.
# Uses ON_ERROR_STOP=0 because some statements use features specific
# to newer PG versions (NULLS NOT DISTINCT on PG15+, NOT ENFORCED on
# PG18+); they fail to apply on older versions but the round-trip
# property still holds because pgc emits SQL based on dumps, not on
# the source schema files.
# 4. Runs pgc compare to produce `output.sql`.
# 5. Applies `output.sql` to `a`.
# 6. Re-runs pgc compare. The new diff must be empty — that's the
# idempotency contract pgc must satisfy.
strategy:
fail-fast: false
matrix:
pg_version: ["14", "15", "16", "17", "18"]

steps:
- uses: actions/checkout@v4

- name: Install Rust
uses: dtolnay/rust-toolchain@stable
with:
toolchain: "1.95.0"

- name: Install PostgreSQL client
run: |
sudo apt-get update
sudo apt-get install -y postgresql-client

- name: Start PostgreSQL ${{ matrix.pg_version }}
run: |
# `services:` does not let us pass `command:` to the container, and
# `wal_level=logical` (required by the publication test cases) needs
# to be set at server start, so we run docker manually.
docker run -d \
--name pg \
-e POSTGRES_PASSWORD=postgres \
-e POSTGRES_USER=postgres \
-p 5432:5432 \
postgres:${{ matrix.pg_version }} \
-c wal_level=logical
# Wait for the server to accept connections.
for i in $(seq 1 60); do
if docker exec pg pg_isready -U postgres -h 127.0.0.1 >/dev/null 2>&1; then
echo "PostgreSQL ${{ matrix.pg_version }} ready after ${i}s"
exit 0
fi
sleep 1
done
echo "PostgreSQL did not become ready in time"
docker logs pg
exit 1

- name: Create test databases
env:
PGPASSWORD: postgres
run: |
psql -h 127.0.0.1 -U postgres -c "CREATE DATABASE a;"
psql -h 127.0.0.1 -U postgres -c "CREATE DATABASE b;"

- name: Apply schemas
env:
PGPASSWORD: postgres
run: |
psql -h 127.0.0.1 -U postgres -d a -v ON_ERROR_STOP=0 \
-f data/test/schema_a.sql
psql -h 127.0.0.1 -U postgres -d b -v ON_ERROR_STOP=0 \
-f data/test/schema_b.sql

- name: Build pgc (release)
run: |
cd ${{ github.workspace }}/app
cargo build --release

- name: Generate pgc config
run: |
cat > /tmp/pgc.conf <<'EOF'
FROM_HOST=127.0.0.1
FROM_PORT=5432
FROM_USER=postgres
FROM_PASSWORD=postgres
FROM_DATABASE=a
FROM_SCHEME=test_schema|shared_schema|new_reporting_schema|data
FROM_SSL=false
FROM_DUMP=/tmp/a.dump
TO_HOST=127.0.0.1
TO_PORT=5432
TO_USER=postgres
TO_PASSWORD=postgres
TO_DATABASE=b
TO_SCHEME=test_schema|shared_schema|new_reporting_schema|data
TO_SSL=false
TO_DUMP=/tmp/b.dump
OUTPUT=/tmp/output.sql
USE_DROP=true
USE_COMMENTS=false
GRANTS_MODE=full
MAX_CONNECTIONS=10
USE_SINGLE_TRANSACTION=false
EOF

- name: Run pgc compare (round 1) — generate diff
run: |
./app/target/release/pgc --config /tmp/pgc.conf
echo "--- output.sql (round 1) size: $(wc -c < /tmp/output.sql) bytes"

- name: Apply diff to database `a`
env:
PGPASSWORD: postgres
run: |
# ON_ERROR_STOP=0 mirrors the schema-apply step above: a diff that
# references PG-version-specific syntax may not apply on every
# version, but as long as the round-trip closes (round-2 diff is
# empty) the test still validates idempotency.
psql -h 127.0.0.1 -U postgres -d a -v ON_ERROR_STOP=0 \
-f /tmp/output.sql

- name: Run pgc compare (round 2) — must be empty
run: |
./app/target/release/pgc --config /tmp/pgc.conf
size=$(wc -c < /tmp/output.sql)
echo "--- output.sql (round 2) size: ${size} bytes"
# With USE_COMMENTS=false a fully-idempotent diff produces a 0-byte
# file; a non-empty file (even one containing only whitespace) means
# the comparer emitted SQL that wasn't covered by round 1.
if [ "${size}" -gt 0 ] && [ -n "$(tr -d '[:space:]' < /tmp/output.sql)" ]; then
echo "ERROR: round-2 diff is not empty on PG ${{ matrix.pg_version }}"
echo "--- output.sql contents: ---"
cat /tmp/output.sql
exit 1
fi
echo "OK: pgc is idempotent on PG ${{ matrix.pg_version }}"

- name: Stop PostgreSQL
if: always()
run: |
docker logs pg | tail -200 || true
docker stop pg || true
docker rm pg || true

docker:
needs: [build, integration]
runs-on: ubuntu-latest
# Only run automatically on master branch, or when manually triggered
if: github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch'

Expand Down
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,6 @@ docker-compose.override.yml
*.dump

.claude/
CLAUDE.md
CLAUDE.md

test.sh
128 changes: 128 additions & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,131 @@
2026-04-25 v1.0.18

Bug fixes:
- Fixed grants for former owners after an owner change
(two-stage fix). The original v1.0.17 behaviour
filtered both old and new owners as implicit
privilege holders, silently dropping explicit grants
to the previous owner from the diff. The first
attempt switched to filtering only the `to` owner —
which exposed a non-idempotent oscillation: PG
materialises an implicit-owner ACL row in
`pg_class.relacl` (e.g. `pgc_owner_from=arwdDxt/
pgc_owner_from`) and treating it as a regular
grantee made the comparer emit a long REVOKE on
first run, then GRANT statements on every
subsequent run. The fix splits owner filtering into
asymmetric `from_owners` / `to_owners` lists that
model the effect of `ALTER ... OWNER TO`: the FROM
owner's implicit-owner row is stripped from the
FROM side (because `ALTER OWNER` removes it) and
the TO owner's implicit row is stripped from the
TO side (because `ALTER OWNER` recreates it). When
ownership does not change both lists hold the same
single role, collapsing back to the original
behaviour. Column ACLs (`pg_attribute.attacl`) pass
an empty `from_owners` because `ALTER TABLE OWNER`
does not touch column-level grants — explicit
column grants to a former owner persist and must
still be revoked under `full` mode.
- `FOREIGN TABLE` ACL parsing now uses the same
privilege set as `TABLE` (SELECT, INSERT, UPDATE,
DELETE, TRUNCATE, REFERENCES, TRIGGER, MAINTAIN).
Previously, foreign-table privileges were parsed
against the default minimal set, which dropped
legitimate privileges from the diff. Foreign-table
grant emission now uses `GRANT ... ON TABLE` rather
than the invalid `GRANT ... ON FOREIGN TABLE`
syntax (PostgreSQL has no `ON FOREIGN TABLE` form
for GRANT/REVOKE — applying the previous diff
raised `syntax error at or near "TABLE"`).
- Foreign-table grants are now idempotent for newly
created foreign tables. PostgreSQL applies default
table privileges (`pg_default_acl.defaclobjtype =
'r'` covers foreign tables too) at CREATE time, so
a TO-only foreign table inherits the FROM database's
default ACL on the freshly created object. Without
adjusting the comparer's effective `from_acl`, a
second compare run after applying the migration
emitted spurious `REVOKE` statements for those
auto-applied privileges. The foreign-table grant
path now mirrors the regular-table path: under
`full` mode, new foreign tables use the FROM
default-table ACL as the effective `from_acl` so
any required revokes are emitted in the same diff
run.
- ACL parser now handles quoted role names with
embedded `=`, `/`, or escaped `""` quotes (e.g.
`"weird=name"=r/owner`). The previous naive
`find('=')` / `find('/')` split misparsed these and
silently corrupted the grantee/grantor strings.
- Dependency-scan needles in the comparer (used to
decide drop/create order for views and routines)
now strip quotes when matching against the
quote-stripped haystack flavour. Dump fields
populated via `quote_ident` literally embed `"` for
mixed-case identifiers, and the previous code could
not match a quoted needle against an unquoted
reference in a definition body — silently losing
real dependency edges.
- `--use-drop=false` is now honoured uniformly for
collations, text-search configurations, text-search
dictionaries, casts, operators, default privileges,
publications, subscriptions, foreign data wrappers,
foreign servers, and user mappings. These eleven
object kinds previously *omitted* the DROP entirely
under `use_drop=false`; they now emit a commented
DROP, matching the behaviour already in place for
tables, views, types, enums, sequences, routines,
rules, event triggers, and foreign tables.

Reliability:
- `current_setting('server_version_num')` is now
fetched once at the start of `Dump::fill` and
threaded into the per-object-kind futures. Previously
it was queried separately by the type and table
branches, doubling the round-trip cost on every dump.
- Unparseable ACL items are now logged with a
`Warning:` line. Previously a silently-skipped
grantee would simply disappear from the diff if a
future PostgreSQL version introduced an aclitem
syntax pgc did not yet recognise.
- `Config::load` now warns when `FROM_HOST` or
`TO_HOST` is empty. The same-target warning gated
on both hosts being non-empty, so a config with
missing host keys silently fell back to defaults.
- `get_schemas` collapsed an `if let Err` followed by
`result.unwrap()` into a single `?`-chain, removing
a redundant unwrap on a Result that was just
pattern-matched.

Internals:
- SEQUENCE ACL parsing explicitly documents that
MAINTAIN ('m', PG17+) is filtered out — MAINTAIN is
only valid on tables, views, materialised views,
and foreign tables, never sequences.
- `to_owners` semantics are documented on
`generate_column_grants_script` (extending the
doc already added to `diff_acls`): current owners
are skipped because they have implicit access;
former owners stay in the diff as ordinary grantees.

Tests:
- New regression tests covering owner-change grant
propagation, explicit grants to former owners under
`full` mode, and the foreign-table privilege set.
- Quoted-grantee ACL parsing tests for embedded `=`,
`/`, escaped `""`, and quoted grantor.
- Owner-unchanged with explicit owner grant in TO
(must be filtered as implicit).
- SEQUENCE with stray MAINTAIN bit (must be silently
dropped, not emitted as an invalid GRANT).
- Column-grant former-owner full-mode test where TO
has *no* explicit column ACL (must still revoke
from former owner without grants to the new owner).
- Dependency-scan needle tests covering both
quoted-against-unquoted and quoted-against-quoted
references.

2026-04-23 v1.0.17

Bug fixes:
Expand Down
2 changes: 1 addition & 1 deletion app/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion app/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pgc"
version = "1.0.17"
version = "1.0.18"
edition = "2024"
license = "MIT"

Expand Down
Loading
Loading