Fix bug of adding temporal schema to types in alter priviledges of functions

[marioloko]

Fixes #366

[greptile-apps]

Greptile Summary

This PR fixes a bug (#366) where schema-qualified type names (e.g., myschema.entity_kind) were incorrectly preserved inside function/procedure privilege signatures (both Privilege.ObjectName and RevokedDefaultPrivilege.ObjectName) during normalization, causing spurious diffs between the desired state and the inspected database state.

Changes:

• Adds stripSchemaFromFunctionSignature helper in ir/normalize.go that removes same-schema qualifiers from all type references embedded in a function/procedure signature string.
• Extends normalizeSchema to iterate over schema.Privileges and schema.RevokedDefaultPrivileges, calling the new helper for FUNCTION and PROCEDURE object types.
• Adds a full set of test fixtures (old.sql, new.sql, diff.sql, plan.json, plan.sql, plan.txt) under testdata/diff/privilege/grant_function_custom_type/ covering the GRANT/REVOKE flow for a function that takes a custom ENUM type as a parameter.

One concern: The helper uses strings.ReplaceAll(signature, schema+".", "") which is a substring match with no word-boundary anchoring. If the current schema's name happens to be a trailing substring of another schema name used in the same signature (e.g., schema "app" and a cross-schema parameter type "myapp.some_type"), the replacement will corrupt the unrelated qualifier. This is an edge case, but a regex-based approach anchored to parameter delimiters (^|[(,\s]) would be more robust.

Confidence Score: 4/5

• Safe to merge for the common case; one edge-case correctness risk with the naive string replacement worth addressing.
• The fix correctly targets the root cause of issue Bug: Changing privileges of functions with enums include the qualified name of the temporal schema #366 and the pointer mutation in the range loop is sound. The new test data validates the primary scenario. The only concern is a narrow edge case where a schema name is a substring suffix of another schema name involved in the same function signature, which the naive strings.ReplaceAll would mishandle. This doesn't affect the common path and doesn't introduce regressions, but it is a real correctness gap in unusual multi-schema setups.
• ir/normalize.go — specifically the stripSchemaFromFunctionSignature helper and its call sites in normalizeSchema.

Important Files Changed

Filename	Overview
ir/normalize.go	Adds stripSchemaFromFunctionSignature and two new loops in normalizeSchema to strip same-schema qualifiers from Privilege.ObjectName and RevokedDefaultPrivilege.ObjectName for function/procedure entries. The mutation via pointer is correct. One edge-case concern: the naive strings.ReplaceAll(signature, schema+".", "") will incorrectly strip characters from cross-schema type references if the current schema's name appears as a trailing substring of another schema name (e.g., schema "app" vs type from "myapp").
testdata/diff/privilege/grant_function_custom_type/new.sql	Desired-state SQL for the new test scenario: creates a custom ENUM type, a function using that type as a parameter, and grants EXECUTE to app_user.
testdata/diff/privilege/grant_function_custom_type/old.sql	Baseline SQL for the test (no privilege grants), representing a database state before any explicit GRANT/REVOKE on the function.
testdata/diff/privilege/grant_function_custom_type/plan.json	Expected plan output with unqualified type names in function signatures, confirming the normalization produces clean SQL without schema prefixes.
testdata/diff/privilege/grant_function_custom_type/diff.sql	Expected diff SQL: REVOKE + GRANT for create_entity with unqualified entity_kind parameter type.
testdata/diff/privilege/grant_function_custom_type/plan.sql	Expected plan SQL output; matches diff.sql.
testdata/diff/privilege/grant_function_custom_type/plan.txt	Human-readable plan summary for the new test case; looks correct.

Sequence Diagram

sequenceDiagram
    participant Caller as Diff Engine
    participant NI as normalizeIR()
    participant NS as normalizeSchema()
    participant SF as stripSchemaFromFunctionSignature()

    Caller->>NI: normalizeIR(ir)
    loop each schema
        NI->>NS: normalizeSchema(schema)
        NS->>NS: normalize tables, views, functions, types…
        loop each Privilege (FUNCTION/PROCEDURE)
            NS->>SF: stripSchemaFromFunctionSignature(ObjectName, schema.Name)
            SF-->>NS: cleaned ObjectName (schema prefix removed from param types)
        end
        loop each RevokedDefaultPrivilege (FUNCTION/PROCEDURE)
            NS->>SF: stripSchemaFromFunctionSignature(ObjectName, schema.Name)
            SF-->>NS: cleaned ObjectName
        end
        NS-->>NI: normalized schema
    end
    NI-->>Caller: normalized IR (ready for diff)

Loading

_{Reviews (1): Last reviewed commit: "Fix bug of adding temporal schema to typ..." | Re-trigger Greptile}

[greptile-apps]

Naive string replacement may over-strip cross-schema type qualifiers

strings.ReplaceAll(signature, schema+".", "") searches for the raw substring anywhere in signature. If the current schema name appears as a suffix of another schema's name, it will incorrectly mutate types from that other schema.

Concrete example:

• Schema under normalization: "app"
• Parameter type from a different schema: "myapp.some_type"
• "myapp.some_type" contains the substring "app." (starting at position 2)
• After replacement: "my" + "" + "some_type" → "mysome_type" ← incorrect

A safer approach is to anchor the match so it only fires when schema. is preceded by a parameter boundary (open-paren, comma, or space):

func stripSchemaFromFunctionSignature(signature, schema string) string {
	if schema == "" {
		return signature
	}
	// Only replace schema. when preceded by a word boundary (start, '(', ',', or space)
	// to avoid matching schema names that are suffixes of longer schema names.
	quotedSchema := regexp.QuoteMeta(`"` + schema + `".`)
	unquotedSchema := regexp.QuoteMeta(schema + ".")
	boundary := `(^|[(,\s])`
	signature = regexp.MustCompile(boundary+quotedSchema).ReplaceAllStringFunc(signature, func(m string) string {
		return m[:len(m)-len(`"`+schema+`".`)]
	})
	signature = regexp.MustCompile(boundary+unquotedSchema).ReplaceAllStringFunc(signature, func(m string) string {
		return m[:len(m)-len(schema+".")]
	})
	return signature
}

Or more simply, pre-compile the patterns with a leading (?:^|[(,\s]) assertion. The current approach is fine for common schema names that are not substrings of other schema names used in the same function signature, but it is a correctness risk for cross-schema type references in that edge case.

[tianzhou]

Please take a look at review comments.

For test case naming, pls prefix with issue_xxx.

See #352

[Copilot]

Pull request overview

Fixes an IR normalization gap that caused function/procedure privilege signatures to include temporary-schema qualified type names (e.g., pgschema_tmp_... .entity_kind), leading to incorrect/unstable GRANT/REVOKE output in generated plans (Issue #366).

Changes:

• Normalize function/procedure privilege object signatures by stripping same-schema qualifiers from referenced types.
• Add a file-based diff test case covering GRANT/REVOKE on a function that uses a custom enum type.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

File	Description
ir/normalize.go	Strips same-schema qualifiers from function/procedure Privileges and RevokedDefaultPrivileges signatures during IR normalization.
testdata/diff/privilege/grant_function_custom_type/*	Adds regression fixtures ensuring plan output uses unqualified same-schema custom types in function signatures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.