Skip to content

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

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

Open
marioloko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 24 additions & 0 deletions ir/normalize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ func normalizeSchema(schema *Schema) {
for _, typeObj := range schema.Types {
normalizeType(typeObj)
}

// Strip same-schema qualifiers from function/procedure privilege signatures
for _, priv := range schema.Privileges {
if priv.ObjectType == PrivilegeObjectTypeFunction || priv.ObjectType == PrivilegeObjectTypeProcedure {
priv.ObjectName = stripSchemaFromFunctionSignature(priv.ObjectName, schema.Name)
}
}
for _, priv := range schema.RevokedDefaultPrivileges {
if priv.ObjectType == PrivilegeObjectTypeFunction || priv.ObjectType == PrivilegeObjectTypeProcedure {
priv.ObjectName = stripSchemaFromFunctionSignature(priv.ObjectName, schema.Name)
}
}
}

// normalizeTable normalizes table-related objects
Expand Down Expand Up @@ -634,6 +646,18 @@ func stripSchemaPrefix(typeName, prefix string) string {
return typeName
}

// stripSchemaFromFunctionSignature strips same-schema qualifiers from type references
// within a function signature like "func_name(p_name text, p_kind myschema.mytype)".
func stripSchemaFromFunctionSignature(signature, schema string) string {
if schema == "" {
return signature
}
// Replace both quoted and unquoted schema prefixes within the signature
signature = strings.ReplaceAll(signature, `"`+schema+`".`, "")
signature = strings.ReplaceAll(signature, schema+".", "")
return signature
Comment on lines +651 to +658
Copy link

@greptile-apps greptile-apps bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

}

// normalizeTrigger normalizes trigger representation
func normalizeTrigger(trigger *Trigger) {
if trigger == nil {
Expand Down
3 changes: 3 additions & 0 deletions testdata/diff/privilege/grant_function_custom_type/diff.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
REVOKE EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) FROM PUBLIC;

GRANT EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) TO app_user;
16 changes: 16 additions & 0 deletions testdata/diff/privilege/grant_function_custom_type/new.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN
CREATE ROLE app_user;
END IF;
END $$;

CREATE TYPE entity_kind AS ENUM ('person', 'company', 'organization');

CREATE FUNCTION create_entity(p_name text, p_kind entity_kind)
RETURNS uuid
LANGUAGE sql
AS $$ SELECT gen_random_uuid(); $$;

REVOKE ALL ON FUNCTION create_entity(text, entity_kind) FROM PUBLIC;
GRANT EXECUTE ON FUNCTION create_entity(text, entity_kind) TO app_user;
13 changes: 13 additions & 0 deletions testdata/diff/privilege/grant_function_custom_type/old.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN
CREATE ROLE app_user;
END IF;
END $$;

CREATE TYPE entity_kind AS ENUM ('person', 'company', 'organization');

CREATE FUNCTION create_entity(p_name text, p_kind entity_kind)
RETURNS uuid
LANGUAGE sql
AS $$ SELECT gen_random_uuid(); $$;
26 changes: 26 additions & 0 deletions testdata/diff/privilege/grant_function_custom_type/plan.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"version": "1.0.0",
"pgschema_version": "1.7.4",
"created_at": "1970-01-01T00:00:00Z",
"source_fingerprint": {
"hash": "bdbb0798afa7207ece90d61d6b44f6a7625cdcb9578888587c326d9c6ae8d22a"
},
"groups": [
{
"steps": [
{
"sql": "REVOKE EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) FROM PUBLIC;",
"type": "revoked_default_privilege",
"operation": "create",
"path": "revoked_default.FUNCTION.create_entity(p_name text, p_kind entity_kind)"
},
{
"sql": "GRANT EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) TO app_user;",
"type": "privilege",
"operation": "create",
"path": "privileges.FUNCTION.create_entity(p_name text, p_kind entity_kind).app_user"
}
]
}
]
}
3 changes: 3 additions & 0 deletions testdata/diff/privilege/grant_function_custom_type/plan.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
REVOKE EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) FROM PUBLIC;

GRANT EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) TO app_user;
18 changes: 18 additions & 0 deletions testdata/diff/privilege/grant_function_custom_type/plan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Plan: 2 to add.

Summary by type:
privileges: 1 to add
revoked default privileges: 1 to add

Privileges:
+ app_user

Revoked default privileges:
+ create_entity(p_name text, p_kind entity_kind)

DDL to be executed:
--------------------------------------------------

REVOKE EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) FROM PUBLIC;

GRANT EXECUTE ON FUNCTION create_entity(p_name text, p_kind entity_kind) TO app_user;
Loading