diff --git a/ir/normalize.go b/ir/normalize.go index 8edddac2..aeddf102 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -819,28 +819,7 @@ func normalizeExpressionParentheses(expr string) string { expr = fmt.Sprintf("(%s)", expr) } - // Step 2: Remove unnecessary parentheses around function calls within the expression - // Specifically targets patterns like (function_name(...)) -> function_name(...) - // This pattern looks for: - // \( - opening parenthesis - // ([a-zA-Z_][a-zA-Z0-9_]*) - function name (captured) - // \( - opening parenthesis for function call - // ([^)]*) - function arguments (captured, non-greedy to avoid matching nested parens) - // \) - closing parenthesis for function call - // \) - closing parenthesis around the whole function - functionParensRegex := regexp.MustCompile(`\(([a-zA-Z_][a-zA-Z0-9_]*\([^)]*\))\)`) - - // Replace (function(...)) with function(...) - // Keep applying until no more matches to handle nested cases - for { - original := expr - expr = functionParensRegex.ReplaceAllString(expr, "$1") - if expr == original { - break - } - } - - // Step 3: Normalize redundant type casts in function arguments + // Step 2: Normalize redundant type casts in function arguments // Pattern: 'text'::text -> 'text' (removing redundant text cast from literals) // IMPORTANT: Do NOT match when followed by [] (array cast is semantically significant) // e.g., '{nested,key}'::text[] must be preserved as-is diff --git a/ir/normalize_test.go b/ir/normalize_test.go index 5eb31386..f7510387 100644 --- a/ir/normalize_test.go +++ b/ir/normalize_test.go @@ -321,6 +321,75 @@ func TestStripSchemaFromReturnType(t *testing.T) { } } +func TestNormalizeExpressionParentheses(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "simple expression gets wrapped", + input: "tenant_id = 1", + expected: "(tenant_id = 1)", + }, + { + name: "already parenthesized", + input: "(tenant_id = 1)", + expected: "(tenant_id = 1)", + }, + { + name: "nested function call preserved", + input: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))", + expected: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))", + }, + { + name: "simple function call preserved", + input: "(has_scope('admin'))", + expected: "(has_scope('admin'))", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizeExpressionParentheses(tt.input) + if result != tt.expected { + t.Errorf("normalizeExpressionParentheses(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestNormalizePolicyExpression(t *testing.T) { + tests := []struct { + name string + expr string + tableSchema string + expected string + }{ + { + name: "nested function call in policy expression", + expr: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))", + tableSchema: "public", + expected: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))", + }, + { + name: "schema-qualified nested function call", + expr: "(id IN ( SELECT unnest(public.get_user_assigned_projects()) AS unnest))", + tableSchema: "public", + expected: "(id IN ( SELECT unnest(get_user_assigned_projects()) AS unnest))", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizePolicyExpression(tt.expr, tt.tableSchema) + if result != tt.expected { + t.Errorf("normalizePolicyExpression(%q, %q) = %q, want %q", tt.expr, tt.tableSchema, result, tt.expected) + } + }) + } +} + func TestNormalizeCheckClause(t *testing.T) { tests := []struct { name string diff --git a/testdata/diff/create_policy/add_policy/diff.sql b/testdata/diff/create_policy/add_policy/diff.sql index f3227297..e5545017 100644 --- a/testdata/diff/create_policy/add_policy/diff.sql +++ b/testdata/diff/create_policy/add_policy/diff.sql @@ -1,7 +1,7 @@ ALTER TABLE orders ENABLE ROW LEVEL SECURITY; CREATE POLICY orders_user_access ON orders FOR SELECT TO PUBLIC USING (user_id IN ( SELECT users.id FROM users)); -CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer); +CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer); CREATE POLICY admin_only ON users FOR DELETE TO PUBLIC USING (is_admin()); CREATE POLICY "my-policy" ON users FOR INSERT TO PUBLIC WITH CHECK ((role)::text = 'user'); CREATE POLICY "select" ON users FOR SELECT TO PUBLIC USING (true); -CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer); +CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer); diff --git a/testdata/diff/create_policy/add_policy/plan.json b/testdata/diff/create_policy/add_policy/plan.json index e7c27d82..1a4bc12e 100644 --- a/testdata/diff/create_policy/add_policy/plan.json +++ b/testdata/diff/create_policy/add_policy/plan.json @@ -21,7 +21,7 @@ "path": "public.orders.orders_user_access" }, { - "sql": "CREATE POLICY \"UserPolicy\" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);", + "sql": "CREATE POLICY \"UserPolicy\" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);", "type": "table.policy", "operation": "create", "path": "public.users.UserPolicy" @@ -45,7 +45,7 @@ "path": "public.users.select" }, { - "sql": "CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer);", + "sql": "CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer);", "type": "table.policy", "operation": "create", "path": "public.users.user_tenant_isolation" diff --git a/testdata/diff/create_policy/add_policy/plan.sql b/testdata/diff/create_policy/add_policy/plan.sql index e054fda1..7f95e587 100644 --- a/testdata/diff/create_policy/add_policy/plan.sql +++ b/testdata/diff/create_policy/add_policy/plan.sql @@ -2,7 +2,7 @@ ALTER TABLE orders ENABLE ROW LEVEL SECURITY; CREATE POLICY orders_user_access ON orders FOR SELECT TO PUBLIC USING (user_id IN ( SELECT users.id FROM users)); -CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer); +CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer); CREATE POLICY admin_only ON users FOR DELETE TO PUBLIC USING (is_admin()); @@ -10,4 +10,4 @@ CREATE POLICY "my-policy" ON users FOR INSERT TO PUBLIC WITH CHECK ((role)::text CREATE POLICY "select" ON users FOR SELECT TO PUBLIC USING (true); -CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer); +CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer); diff --git a/testdata/diff/create_policy/add_policy/plan.txt b/testdata/diff/create_policy/add_policy/plan.txt index 4e760a23..59f606d2 100644 --- a/testdata/diff/create_policy/add_policy/plan.txt +++ b/testdata/diff/create_policy/add_policy/plan.txt @@ -21,7 +21,7 @@ ALTER TABLE orders ENABLE ROW LEVEL SECURITY; CREATE POLICY orders_user_access ON orders FOR SELECT TO PUBLIC USING (user_id IN ( SELECT users.id FROM users)); -CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer); +CREATE POLICY "UserPolicy" ON users TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer); CREATE POLICY admin_only ON users FOR DELETE TO PUBLIC USING (is_admin()); @@ -29,4 +29,4 @@ CREATE POLICY "my-policy" ON users FOR INSERT TO PUBLIC WITH CHECK ((role)::text CREATE POLICY "select" ON users FOR SELECT TO PUBLIC USING (true); -CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = current_setting('app.current_tenant')::integer); +CREATE POLICY user_tenant_isolation ON users FOR UPDATE TO PUBLIC USING (tenant_id = (current_setting('app.current_tenant'))::integer); diff --git a/testdata/diff/create_policy/alter_policy_roles/plan.json b/testdata/diff/create_policy/alter_policy_roles/plan.json index cc303b13..c779f988 100644 --- a/testdata/diff/create_policy/alter_policy_roles/plan.json +++ b/testdata/diff/create_policy/alter_policy_roles/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.8.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "a24989791c9ba13d2d21f70cd0405e0e509c55341872b552ab87a0c79d238ddd" + "hash": "a5fcd005530df97b095b020e1a8c2fb12333baf056c3a2318bc34f2157893877" }, "groups": [ { diff --git a/testdata/diff/create_policy/alter_policy_using/plan.json b/testdata/diff/create_policy/alter_policy_using/plan.json index 824616c4..9571281c 100644 --- a/testdata/diff/create_policy/alter_policy_using/plan.json +++ b/testdata/diff/create_policy/alter_policy_using/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.8.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "e7aac12e5d350ee9d014b756c838017ddd0c17180b7baa33300f4a773c4c89b1" + "hash": "b23d70f93fe88b9bfcf8d05f46e3faa60d54cf469813e2ead871e9c9d9231ab7" }, "groups": [ { diff --git a/testdata/diff/create_policy/disable_rls/plan.json b/testdata/diff/create_policy/disable_rls/plan.json index bff3fcb6..0803b934 100644 --- a/testdata/diff/create_policy/disable_rls/plan.json +++ b/testdata/diff/create_policy/disable_rls/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.8.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "a24989791c9ba13d2d21f70cd0405e0e509c55341872b552ab87a0c79d238ddd" + "hash": "a5fcd005530df97b095b020e1a8c2fb12333baf056c3a2318bc34f2157893877" }, "groups": [ { diff --git a/testdata/diff/create_policy/drop_policy/plan.json b/testdata/diff/create_policy/drop_policy/plan.json index b9404102..0f00feb8 100644 --- a/testdata/diff/create_policy/drop_policy/plan.json +++ b/testdata/diff/create_policy/drop_policy/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.8.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "a24989791c9ba13d2d21f70cd0405e0e509c55341872b552ab87a0c79d238ddd" + "hash": "a5fcd005530df97b095b020e1a8c2fb12333baf056c3a2318bc34f2157893877" }, "groups": [ { diff --git a/testdata/diff/create_policy/force_rls/plan.json b/testdata/diff/create_policy/force_rls/plan.json index 52d6ab17..bb75c354 100644 --- a/testdata/diff/create_policy/force_rls/plan.json +++ b/testdata/diff/create_policy/force_rls/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.8.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "5e4ac40b64a7ec01d1f41af5c61ba420960e98b8ac166d757ee424ea3f10474f" + "hash": "f26c6614d22a712e9314414c386efca5bb723c12064c63e173483f718716f193" }, "groups": [ { diff --git a/testdata/diff/create_policy/issue_377_nested_function_in_policy/diff.sql b/testdata/diff/create_policy/issue_377_nested_function_in_policy/diff.sql new file mode 100644 index 00000000..407496c5 --- /dev/null +++ b/testdata/diff/create_policy/issue_377_nested_function_in_policy/diff.sql @@ -0,0 +1 @@ +ALTER TABLE projects ADD COLUMN description text; diff --git a/testdata/diff/create_policy/issue_377_nested_function_in_policy/new.sql b/testdata/diff/create_policy/issue_377_nested_function_in_policy/new.sql new file mode 100644 index 00000000..6aecf891 --- /dev/null +++ b/testdata/diff/create_policy/issue_377_nested_function_in_policy/new.sql @@ -0,0 +1,16 @@ +CREATE FUNCTION get_user_assigned_projects() RETURNS integer[] +LANGUAGE sql STABLE AS $$ SELECT ARRAY[1, 2, 3] $$; + +CREATE TABLE projects ( + id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL, + description TEXT +); + +ALTER TABLE projects ENABLE ROW LEVEL SECURITY; + +-- Policy unchanged, but table has new column +CREATE POLICY project_access ON projects + FOR SELECT + TO PUBLIC + USING (id IN (SELECT unnest(get_user_assigned_projects()) AS unnest)); diff --git a/testdata/diff/create_policy/issue_377_nested_function_in_policy/old.sql b/testdata/diff/create_policy/issue_377_nested_function_in_policy/old.sql new file mode 100644 index 00000000..d8445d68 --- /dev/null +++ b/testdata/diff/create_policy/issue_377_nested_function_in_policy/old.sql @@ -0,0 +1,14 @@ +CREATE FUNCTION get_user_assigned_projects() RETURNS integer[] +LANGUAGE sql STABLE AS $$ SELECT ARRAY[1, 2, 3] $$; + +CREATE TABLE projects ( + id SERIAL PRIMARY KEY, + name VARCHAR(100) NOT NULL +); + +ALTER TABLE projects ENABLE ROW LEVEL SECURITY; + +CREATE POLICY project_access ON projects + FOR SELECT + TO PUBLIC + USING (id IN (SELECT unnest(get_user_assigned_projects()) AS unnest)); diff --git a/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.json b/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.json new file mode 100644 index 00000000..9af0ff84 --- /dev/null +++ b/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.json @@ -0,0 +1,20 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.8.0", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "37d3ebdd423c5db41ba042431c6328705f50593fe2c8f5163de497aa4eee08b8" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER TABLE projects ADD COLUMN description text;", + "type": "table.column", + "operation": "create", + "path": "public.projects.description" + } + ] + } + ] +} diff --git a/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.sql b/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.sql new file mode 100644 index 00000000..407496c5 --- /dev/null +++ b/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.sql @@ -0,0 +1 @@ +ALTER TABLE projects ADD COLUMN description text; diff --git a/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.txt b/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.txt new file mode 100644 index 00000000..a2c20220 --- /dev/null +++ b/testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.txt @@ -0,0 +1,13 @@ +Plan: 1 to modify. + +Summary by type: + tables: 1 to modify + +Tables: + ~ projects + + description (column) + +DDL to be executed: +-------------------------------------------------- + +ALTER TABLE projects ADD COLUMN description text; diff --git a/testdata/diff/create_policy/remove_force_rls/plan.json b/testdata/diff/create_policy/remove_force_rls/plan.json index 893f11e6..e6c52c8c 100644 --- a/testdata/diff/create_policy/remove_force_rls/plan.json +++ b/testdata/diff/create_policy/remove_force_rls/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.8.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "0d3cfe3e8283f19f2e5ea37cfc7f2cbab91b543bf77aac99a2a912a17bc6e3f1" + "hash": "6d556186d2ea5db5c7aebfd47e9a7d91430bbed6f2c9052964d2d17424b6984d" }, "groups": [ {