From 3a3a952ee7631bab9c0f0aa8c58ed2dd59437e86 Mon Sep 17 00:00:00 2001 From: Shehjad Date: Sat, 17 Oct 2026 00:42:16 +0600 Subject: [PATCH 1/2] fix: render per-privilege column lists in grants (#638) --- internal/diff/column_privilege.go | 73 ++++++++++--------- .../diff.sql | 5 ++ .../new.sql | 14 ++++ .../old.sql | 14 ++++ .../plan.json | 32 ++++++++ .../plan.sql | 5 ++ .../plan.txt | 18 +++++ 7 files changed, 125 insertions(+), 36 deletions(-) create mode 100644 testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql create mode 100644 testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql create mode 100644 testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql create mode 100644 testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json create mode 100644 testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql create mode 100644 testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt diff --git a/internal/diff/column_privilege.go b/internal/diff/column_privilege.go index 305f0e23b..2e8f87c09 100644 --- a/internal/diff/column_privilege.go +++ b/internal/diff/column_privilege.go @@ -84,20 +84,12 @@ func generateGrantColumnPrivilegeSQL(cp *ir.ColumnPrivilege) string { copy(sortedPrivs, cp.Privileges) sort.Strings(sortedPrivs) - privStr := strings.Join(sortedPrivs, ", ") - - // Format columns with proper quoting - quotedCols := make([]string, len(cp.Columns)) - for i, col := range cp.Columns { - quotedCols[i] = ir.QuoteIdentifier(col) - } - sort.Strings(quotedCols) - colStr := strings.Join(quotedCols, ", ") + privStr := formatColumnPrivilegeList(sortedPrivs, formatColumnList(cp.Columns)) grantee := formatGrantee(cp.Grantee) tableName := ir.QuoteIdentifier(cp.TableName) - sql := fmt.Sprintf("GRANT %s (%s) ON TABLE %s TO %s", privStr, colStr, tableName, grantee) + sql := fmt.Sprintf("GRANT %s ON TABLE %s TO %s", privStr, tableName, grantee) if cp.WithGrantOption { sql += " WITH GRANT OPTION" @@ -113,20 +105,12 @@ func generateRevokeColumnPrivilegeSQL(cp *ir.ColumnPrivilege) string { copy(sortedPrivs, cp.Privileges) sort.Strings(sortedPrivs) - privStr := strings.Join(sortedPrivs, ", ") - - // Format columns with proper quoting - quotedCols := make([]string, len(cp.Columns)) - for i, col := range cp.Columns { - quotedCols[i] = ir.QuoteIdentifier(col) - } - sort.Strings(quotedCols) - colStr := strings.Join(quotedCols, ", ") + privStr := formatColumnPrivilegeList(sortedPrivs, formatColumnList(cp.Columns)) grantee := formatGrantee(cp.Grantee) tableName := ir.QuoteIdentifier(cp.TableName) - return fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s;", privStr, colStr, tableName, grantee) + return fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s;", privStr, tableName, grantee) } // generateAlterColumnPrivilegeStatements generates statements for column privilege modifications @@ -160,26 +144,20 @@ func (d *columnPrivilegeDiff) generateAlterColumnPrivilegeStatements() []string grantee := formatGrantee(d.New.Grantee) tableName := ir.QuoteIdentifier(d.New.TableName) - // Format columns with proper quoting - quotedCols := make([]string, len(d.New.Columns)) - for i, col := range d.New.Columns { - quotedCols[i] = ir.QuoteIdentifier(col) - } - sort.Strings(quotedCols) - colStr := strings.Join(quotedCols, ", ") + colStr := formatColumnList(d.New.Columns) // Generate REVOKE for removed privileges if len(toRevoke) > 0 { sort.Strings(toRevoke) - statements = append(statements, fmt.Sprintf("REVOKE %s (%s) ON TABLE %s FROM %s;", - strings.Join(toRevoke, ", "), colStr, tableName, grantee)) + statements = append(statements, fmt.Sprintf("REVOKE %s ON TABLE %s FROM %s;", + formatColumnPrivilegeList(toRevoke, colStr), tableName, grantee)) } // Generate GRANT for added privileges if len(toGrant) > 0 { sort.Strings(toGrant) - sql := fmt.Sprintf("GRANT %s (%s) ON TABLE %s TO %s", - strings.Join(toGrant, ", "), colStr, tableName, grantee) + sql := fmt.Sprintf("GRANT %s ON TABLE %s TO %s", + formatColumnPrivilegeList(toGrant, colStr), tableName, grantee) if d.New.WithGrantOption { sql += " WITH GRANT OPTION" } @@ -198,16 +176,16 @@ func (d *columnPrivilegeDiff) generateAlterColumnPrivilegeStatements() []string if len(unchanged) > 0 { sort.Strings(unchanged) - unchangedStr := strings.Join(unchanged, ", ") + unchangedStr := formatColumnPrivilegeList(unchanged, colStr) if d.Old.WithGrantOption && !d.New.WithGrantOption { // Revoke grant option only (keep the privilege) - statements = append(statements, fmt.Sprintf("REVOKE GRANT OPTION FOR %s (%s) ON TABLE %s FROM %s;", - unchangedStr, colStr, tableName, grantee)) + statements = append(statements, fmt.Sprintf("REVOKE GRANT OPTION FOR %s ON TABLE %s FROM %s;", + unchangedStr, tableName, grantee)) } else if !d.Old.WithGrantOption && d.New.WithGrantOption { // Add grant option (re-grant with grant option) - statements = append(statements, fmt.Sprintf("GRANT %s (%s) ON TABLE %s TO %s WITH GRANT OPTION;", - unchangedStr, colStr, tableName, grantee)) + statements = append(statements, fmt.Sprintf("GRANT %s ON TABLE %s TO %s WITH GRANT OPTION;", + unchangedStr, tableName, grantee)) } } } @@ -262,3 +240,26 @@ func columnPrivilegesEqual(old, new *ir.ColumnPrivilege) bool { func (d *columnPrivilegeDiff) GetObjectName() string { return d.New.GetObjectKey() } + +// formatColumnList quotes and sorts column names and joins them for use in a +// column privilege list, e.g. "a, b". +func formatColumnList(columns []string) string { + quotedCols := make([]string, len(columns)) + for i, col := range columns { + quotedCols[i] = ir.QuoteIdentifier(col) + } + sort.Strings(quotedCols) + return strings.Join(quotedCols, ", ") +} + +// formatColumnPrivilegeList attaches the column list to every privilege, e.g. +// "INSERT (a, b), UPDATE (a, b)". PostgreSQL applies a column list only to the +// privilege immediately before it, so "INSERT, UPDATE (a, b)" would grant +// INSERT on the whole table (#638). +func formatColumnPrivilegeList(privileges []string, colStr string) string { + parts := make([]string, len(privileges)) + for i, p := range privileges { + parts[i] = fmt.Sprintf("%s (%s)", p, colStr) + } + return strings.Join(parts, ", ") +} diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql new file mode 100644 index 000000000..c07061bad --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql @@ -0,0 +1,5 @@ +REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user; + +GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user; + +GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql new file mode 100644 index 000000000..786ddec59 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql @@ -0,0 +1,14 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +CREATE TABLE t (id int PRIMARY KEY, a int, b int); + +CREATE TABLE u (id int PRIMARY KEY, x int); + +GRANT INSERT (a), UPDATE (a) ON t TO app_user; + +GRANT INSERT (x), SELECT (x), UPDATE (x) ON u TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql new file mode 100644 index 000000000..d187083fe --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql @@ -0,0 +1,14 @@ +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +CREATE TABLE t (id int PRIMARY KEY, a int, b int); + +CREATE TABLE u (id int PRIMARY KEY, x int); + +GRANT INSERT (a, b), UPDATE (a, b) ON t TO app_user; + +GRANT SELECT (x) ON u TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json new file mode 100644 index 000000000..b0e840009 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json @@ -0,0 +1,32 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.13.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "e248c37b13f3d75676e614591a4b50cfbc09461787e556cb08e292793cd044a3" + }, + "groups": [ + { + "steps": [ + { + "sql": "REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user;", + "type": "column_privilege", + "operation": "drop", + "path": "column_privileges.TABLE.t.a,b.app_user" + }, + { + "sql": "GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user;", + "type": "column_privilege", + "operation": "alter", + "path": "column_privileges.TABLE.u.x.app_user" + }, + { + "sql": "GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user;", + "type": "column_privilege", + "operation": "create", + "path": "column_privileges.TABLE.t.a.app_user" + } + ] + } + ] +} diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql new file mode 100644 index 000000000..c07061bad --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql @@ -0,0 +1,5 @@ +REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user; + +GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user; + +GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt new file mode 100644 index 000000000..885fd2f09 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt @@ -0,0 +1,18 @@ +Plan: 1 to add, 1 to modify, 1 to drop. + +Summary by type: + column privileges: 1 to add, 1 to modify, 1 to drop + +Column privileges: + - app_user + + app_user + ~ app_user + +DDL to be executed: +-------------------------------------------------- + +REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user; + +GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user; + +GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user; From 5d34ab877fef4ac333810fef798ee86fd1831c78 Mon Sep 17 00:00:00 2001 From: Shehjad Date: Sat, 17 Oct 2026 01:16:34 +0600 Subject: [PATCH 2/2] test: cover grant option changes for multi-privilege column grants (#638) --- .../diff.sql | 4 ++++ .../issue_638_multi_privilege_column_grant/new.sql | 8 ++++++++ .../issue_638_multi_privilege_column_grant/old.sql | 8 ++++++++ .../plan.json | 14 +++++++++++++- .../plan.sql | 4 ++++ .../plan.txt | 10 ++++++++-- 6 files changed, 45 insertions(+), 3 deletions(-) diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql index c07061bad..190a32471 100644 --- a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql @@ -2,4 +2,8 @@ REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user; GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user; +GRANT INSERT (c), UPDATE (c) ON TABLE v TO app_user WITH GRANT OPTION; + +REVOKE GRANT OPTION FOR INSERT (c), UPDATE (c) ON TABLE w FROM app_user; + GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql index 786ddec59..eeee211cc 100644 --- a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql @@ -9,6 +9,14 @@ CREATE TABLE t (id int PRIMARY KEY, a int, b int); CREATE TABLE u (id int PRIMARY KEY, x int); +CREATE TABLE v (id int PRIMARY KEY, c int); + +CREATE TABLE w (id int PRIMARY KEY, c int); + GRANT INSERT (a), UPDATE (a) ON t TO app_user; GRANT INSERT (x), SELECT (x), UPDATE (x) ON u TO app_user; + +GRANT INSERT (c), UPDATE (c) ON v TO app_user WITH GRANT OPTION; + +GRANT INSERT (c), UPDATE (c) ON w TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql index d187083fe..a983c4e95 100644 --- a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql @@ -9,6 +9,14 @@ CREATE TABLE t (id int PRIMARY KEY, a int, b int); CREATE TABLE u (id int PRIMARY KEY, x int); +CREATE TABLE v (id int PRIMARY KEY, c int); + +CREATE TABLE w (id int PRIMARY KEY, c int); + GRANT INSERT (a, b), UPDATE (a, b) ON t TO app_user; GRANT SELECT (x) ON u TO app_user; + +GRANT INSERT (c), UPDATE (c) ON v TO app_user; + +GRANT INSERT (c), UPDATE (c) ON w TO app_user WITH GRANT OPTION; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json index b0e840009..bba54e1a8 100644 --- a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.13.3", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "e248c37b13f3d75676e614591a4b50cfbc09461787e556cb08e292793cd044a3" + "hash": "ca8fed2741083ff2d176cec3b66b587bcf4d1a0d50c25519579134aaa173c093" }, "groups": [ { @@ -20,6 +20,18 @@ "operation": "alter", "path": "column_privileges.TABLE.u.x.app_user" }, + { + "sql": "GRANT INSERT (c), UPDATE (c) ON TABLE v TO app_user WITH GRANT OPTION;", + "type": "column_privilege", + "operation": "alter", + "path": "column_privileges.TABLE.v.c.app_user" + }, + { + "sql": "REVOKE GRANT OPTION FOR INSERT (c), UPDATE (c) ON TABLE w FROM app_user;", + "type": "column_privilege", + "operation": "alter", + "path": "column_privileges.TABLE.w.c.app_user" + }, { "sql": "GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user;", "type": "column_privilege", diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql index c07061bad..190a32471 100644 --- a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql @@ -2,4 +2,8 @@ REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user; GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user; +GRANT INSERT (c), UPDATE (c) ON TABLE v TO app_user WITH GRANT OPTION; + +REVOKE GRANT OPTION FOR INSERT (c), UPDATE (c) ON TABLE w FROM app_user; + GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user; diff --git a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt index 885fd2f09..c8515df53 100644 --- a/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt @@ -1,12 +1,14 @@ -Plan: 1 to add, 1 to modify, 1 to drop. +Plan: 1 to add, 3 to modify, 1 to drop. Summary by type: - column privileges: 1 to add, 1 to modify, 1 to drop + column privileges: 1 to add, 3 to modify, 1 to drop Column privileges: - app_user + app_user ~ app_user + ~ app_user + ~ app_user DDL to be executed: -------------------------------------------------- @@ -15,4 +17,8 @@ REVOKE INSERT (a, b), UPDATE (a, b) ON TABLE t FROM app_user; GRANT INSERT (x), UPDATE (x) ON TABLE u TO app_user; +GRANT INSERT (c), UPDATE (c) ON TABLE v TO app_user WITH GRANT OPTION; + +REVOKE GRANT OPTION FOR INSERT (c), UPDATE (c) ON TABLE w FROM app_user; + GRANT INSERT (a), UPDATE (a) ON TABLE t TO app_user;