diff --git a/internal/diff/column_privilege.go b/internal/diff/column_privilege.go index 305f0e23..2e8f87c0 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 00000000..190a3247 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/diff.sql @@ -0,0 +1,9 @@ +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 new file mode 100644 index 00000000..eeee211c --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/new.sql @@ -0,0 +1,22 @@ +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); + +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 new file mode 100644 index 00000000..a983c4e9 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/old.sql @@ -0,0 +1,22 @@ +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); + +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 new file mode 100644 index 00000000..bba54e1a --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.json @@ -0,0 +1,44 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.13.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "ca8fed2741083ff2d176cec3b66b587bcf4d1a0d50c25519579134aaa173c093" + }, + "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 (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", + "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 00000000..190a3247 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.sql @@ -0,0 +1,9 @@ +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 new file mode 100644 index 00000000..c8515df5 --- /dev/null +++ b/testdata/diff/privilege/issue_638_multi_privilege_column_grant/plan.txt @@ -0,0 +1,24 @@ +Plan: 1 to add, 3 to modify, 1 to drop. + +Summary by type: + 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: +-------------------------------------------------- + +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;