diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index 3e0883cb..cb21bad3 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -102,6 +102,13 @@ func TestDumpCommand_Issue275TruncatedFunctionGrants(t *testing.T) { runExactMatchTest(t, "issue_275_truncated_function_grants") } +func TestDumpCommand_Issue252FunctionSchemaQualifier(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + runExactMatchTest(t, "issue_252_function_schema_qualifier") +} + func runExactMatchTest(t *testing.T, testDataDir string) { runExactMatchTestWithContext(t, context.Background(), testDataDir) } diff --git a/ir/normalize.go b/ir/normalize.go index 07ea054c..8ac9af91 100644 --- a/ir/normalize.go +++ b/ir/normalize.go @@ -313,6 +313,8 @@ func normalizeFunction(function *Function) { } // Normalize function body to handle whitespace differences function.Definition = normalizeFunctionDefinition(function.Definition) + // Strip current schema qualifier from function body for consistent unqualified output + function.Definition = stripSchemaPrefixFromBody(function.Definition, function.Schema) } // normalizeFunctionDefinition normalizes function body whitespace @@ -334,6 +336,68 @@ func normalizeFunctionDefinition(def string) string { return strings.Join(normalized, "\n") } +// stripSchemaPrefixFromBody removes the current schema qualifier from identifiers +// in a function or procedure body. For example, "public.users" becomes "users". +// It skips single-quoted string literals to avoid modifying string constants. +func stripSchemaPrefixFromBody(body, schema string) string { + if body == "" || schema == "" { + return body + } + + prefix := schema + "." + prefixLen := len(prefix) + + // Fast path: if the prefix doesn't appear at all, return as-is + if !strings.Contains(body, prefix) { + return body + } + + var result strings.Builder + result.Grow(len(body)) + inString := false + + for i := 0; i < len(body); i++ { + ch := body[i] + + // Track single-quoted string literals, handling '' escapes + if ch == '\'' { + if inString { + if i+1 < len(body) && body[i+1] == '\'' { + // Escaped quote inside string: write both and skip + result.WriteString("''") + i++ + continue + } + inString = false + } else { + inString = true + } + result.WriteByte(ch) + continue + } + + // Only attempt replacement outside string literals + if !inString && i+prefixLen <= len(body) && body[i:i+prefixLen] == prefix { + // Ensure this is a schema qualifier, not part of a longer identifier + // (e.g., "not_public.users" should not match) + if i == 0 || !isIdentChar(body[i-1]) { + // Skip the schema prefix, keep everything after it + i += prefixLen - 1 + continue + } + } + + result.WriteByte(ch) + } + + return result.String() +} + +// isIdentChar returns true if the byte is a valid SQL identifier character. +func isIdentChar(b byte) bool { + return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') || b == '_' +} + // normalizeProcedure normalizes procedure representation func normalizeProcedure(procedure *Procedure) { if procedure == nil { @@ -358,6 +422,9 @@ func normalizeProcedure(procedure *Procedure) { } } } + + // Strip current schema qualifier from procedure body for consistent unqualified output + procedure.Definition = stripSchemaPrefixFromBody(procedure.Definition, procedure.Schema) } // normalizeFunctionReturnType normalizes function return types, especially TABLE types diff --git a/ir/normalize_test.go b/ir/normalize_test.go index 528de4d5..b4f4ef77 100644 --- a/ir/normalize_test.go +++ b/ir/normalize_test.go @@ -4,6 +4,103 @@ import ( "testing" ) +func TestStripSchemaPrefixFromBody(t *testing.T) { + tests := []struct { + name string + body string + schema string + expected string + }{ + { + name: "empty body", + body: "", + schema: "public", + expected: "", + }, + { + name: "empty schema", + body: "SELECT * FROM public.users", + schema: "", + expected: "SELECT * FROM public.users", + }, + { + name: "no match", + body: "SELECT * FROM users", + schema: "public", + expected: "SELECT * FROM users", + }, + { + name: "simple table reference", + body: "SELECT * FROM public.users", + schema: "public", + expected: "SELECT * FROM users", + }, + { + name: "multiple references", + body: "INSERT INTO public.users SELECT * FROM public.accounts WHERE public.accounts.id > 0", + schema: "public", + expected: "INSERT INTO users SELECT * FROM accounts WHERE accounts.id > 0", + }, + { + name: "preserves string literal", + body: "RETURN 'Table: public.users'", + schema: "public", + expected: "RETURN 'Table: public.users'", + }, + { + name: "preserves escaped quotes in string", + body: "RETURN 'it''s public.users here'", + schema: "public", + expected: "RETURN 'it''s public.users here'", + }, + { + name: "strips outside but preserves inside string", + body: "SELECT public.users.id, 'public.users' FROM public.users", + schema: "public", + expected: "SELECT users.id, 'public.users' FROM users", + }, + { + name: "does not match partial identifier", + body: "SELECT * FROM not_public.users", + schema: "public", + expected: "SELECT * FROM not_public.users", + }, + { + name: "different schema not stripped", + body: "SELECT * FROM other_schema.users", + schema: "public", + expected: "SELECT * FROM other_schema.users", + }, + { + name: "type cast with schema", + body: "SELECT x::public.my_type FROM public.users", + schema: "public", + expected: "SELECT x::my_type FROM users", + }, + { + name: "start of body", + body: "public.users WHERE id = 1", + schema: "public", + expected: "users WHERE id = 1", + }, + { + name: "plpgsql function body", + body: "\nBEGIN\n RETURN (SELECT count(*)::integer FROM public.users);\nEND;\n", + schema: "public", + expected: "\nBEGIN\n RETURN (SELECT count(*)::integer FROM users);\nEND;\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := stripSchemaPrefixFromBody(tt.body, tt.schema) + if result != tt.expected { + t.Errorf("stripSchemaPrefixFromBody(%q, %q) = %q, want %q", tt.body, tt.schema, result, tt.expected) + } + }) + } +} + func TestNormalizeCheckClause(t *testing.T) { tests := []struct { name string diff --git a/testdata/diff/create_trigger/add_trigger/plan.json b/testdata/diff/create_trigger/add_trigger/plan.json index b9403166..5c7fa0f9 100644 --- a/testdata/diff/create_trigger/add_trigger/plan.json +++ b/testdata/diff/create_trigger/add_trigger/plan.json @@ -3,7 +3,7 @@ "pgschema_version": "1.7.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { - "hash": "3f73c416e0d69d8eaf60edf93de71971e02d53d0750d912f069eba5c0b394329" + "hash": "b640a56ddb8d22697eeec4c5a6db0ebf41b14d2d11089187b3fa71b95e8e2d87" }, "groups": [ { @@ -31,12 +31,6 @@ "type": "view.trigger", "operation": "create", "path": "public.employee_emails.trg_employee_emails_insert" - }, - { - "sql": "CREATE OR REPLACE FUNCTION insert_employee_emails()\nRETURNS trigger\nLANGUAGE plpgsql\nVOLATILE\nAS $$\nBEGIN\n INSERT INTO employees (name)\n VALUES (NEW.name)\n RETURNING id, name INTO NEW.id, NEW.name;\n RETURN NEW;\nEND;\n$$;", - "type": "function", - "operation": "alter", - "path": "public.insert_employee_emails" } ] } diff --git a/testdata/diff/create_trigger/add_trigger/plan.sql b/testdata/diff/create_trigger/add_trigger/plan.sql index 75302ca5..e8b5b456 100644 --- a/testdata/diff/create_trigger/add_trigger/plan.sql +++ b/testdata/diff/create_trigger/add_trigger/plan.sql @@ -17,16 +17,3 @@ CREATE OR REPLACE TRIGGER trg_employee_emails_insert INSTEAD OF INSERT ON employee_emails FOR EACH ROW EXECUTE FUNCTION insert_employee_emails(); - -CREATE OR REPLACE FUNCTION insert_employee_emails() -RETURNS trigger -LANGUAGE plpgsql -VOLATILE -AS $$ -BEGIN - INSERT INTO employees (name) - VALUES (NEW.name) - RETURNING id, name INTO NEW.id, NEW.name; - RETURN NEW; -END; -$$; diff --git a/testdata/diff/create_trigger/add_trigger/plan.txt b/testdata/diff/create_trigger/add_trigger/plan.txt index c34d68da..957b8fa7 100644 --- a/testdata/diff/create_trigger/add_trigger/plan.txt +++ b/testdata/diff/create_trigger/add_trigger/plan.txt @@ -1,13 +1,9 @@ -Plan: 3 to modify. +Plan: 2 to modify. Summary by type: - functions: 1 to modify tables: 1 to modify views: 1 to modify -Functions: - ~ insert_employee_emails - Tables: ~ employees + employees_insert_timestamp_trigger (trigger) @@ -40,16 +36,3 @@ CREATE OR REPLACE TRIGGER trg_employee_emails_insert INSTEAD OF INSERT ON employee_emails FOR EACH ROW EXECUTE FUNCTION insert_employee_emails(); - -CREATE OR REPLACE FUNCTION insert_employee_emails() -RETURNS trigger -LANGUAGE plpgsql -VOLATILE -AS $$ -BEGIN - INSERT INTO employees (name) - VALUES (NEW.name) - RETURNING id, name INTO NEW.id, NEW.name; - RETURN NEW; -END; -$$; diff --git a/testdata/diff/create_trigger/add_trigger_constraint/plan.json b/testdata/diff/create_trigger/add_trigger_constraint/plan.json index bca6aa13..6dd616f0 100644 --- a/testdata/diff/create_trigger/add_trigger_constraint/plan.json +++ b/testdata/diff/create_trigger/add_trigger_constraint/plan.json @@ -1,6 +1,6 @@ { "version": "1.0.0", - "pgschema_version": "1.6.2", + "pgschema_version": "1.7.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { "hash": "78dc0b05e73aa55e80425103b0d8fb983f32eb26cae6db2f6c293661b8b60f18" diff --git a/testdata/diff/create_trigger/add_trigger_old_table/plan.json b/testdata/diff/create_trigger/add_trigger_old_table/plan.json index 1e11d24f..1da01f7d 100644 --- a/testdata/diff/create_trigger/add_trigger_old_table/plan.json +++ b/testdata/diff/create_trigger/add_trigger_old_table/plan.json @@ -1,6 +1,6 @@ { "version": "1.0.0", - "pgschema_version": "1.6.2", + "pgschema_version": "1.7.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { "hash": "fdad24e9951fe452a5564a5b7b9de8f12531310dd442f752cc393e3fc9a28c85" diff --git a/testdata/diff/create_trigger/add_trigger_system_catalog/plan.json b/testdata/diff/create_trigger/add_trigger_system_catalog/plan.json index c79aea95..51b3361a 100644 --- a/testdata/diff/create_trigger/add_trigger_system_catalog/plan.json +++ b/testdata/diff/create_trigger/add_trigger_system_catalog/plan.json @@ -1,6 +1,6 @@ { "version": "1.0.0", - "pgschema_version": "1.6.2", + "pgschema_version": "1.7.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { "hash": "b12c4184141eaba7448393d463064658c09818c4be2d97bbd9a28cd0567ff9c8" diff --git a/testdata/diff/create_trigger/add_trigger_when_distinct/plan.json b/testdata/diff/create_trigger/add_trigger_when_distinct/plan.json index d90de822..55baa927 100644 --- a/testdata/diff/create_trigger/add_trigger_when_distinct/plan.json +++ b/testdata/diff/create_trigger/add_trigger_when_distinct/plan.json @@ -1,6 +1,6 @@ { "version": "1.0.0", - "pgschema_version": "1.6.2", + "pgschema_version": "1.7.0", "created_at": "1970-01-01T00:00:00Z", "source_fingerprint": { "hash": "ad4075d6c365b628b72e784861c986b1bed0f7c395266c5acc4f52aab3f3fed9" diff --git a/testdata/diff/dependency/table_to_function/diff.sql b/testdata/diff/dependency/table_to_function/diff.sql index 22faba8c..18a1e39c 100644 --- a/testdata/diff/dependency/table_to_function/diff.sql +++ b/testdata/diff/dependency/table_to_function/diff.sql @@ -13,6 +13,6 @@ LANGUAGE plpgsql VOLATILE AS $$ BEGIN - RETURN (SELECT COUNT(*) FROM public.documents); + RETURN (SELECT COUNT(*) FROM documents); END; $$; diff --git a/testdata/dump/issue_252_function_schema_qualifier/manifest.json b/testdata/dump/issue_252_function_schema_qualifier/manifest.json new file mode 100644 index 00000000..0c7d02ca --- /dev/null +++ b/testdata/dump/issue_252_function_schema_qualifier/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "issue_252_function_schema_qualifier", + "description": "Test case for schema qualifiers in function/procedure bodies not being stripped in dump (GitHub issue #252)", + "source": "https://github.com/pgplex/pgschema/issues/252", + "notes": [ + "Reproduces the bug where function bodies retain schema qualifiers (e.g., public.users) while table definitions are unqualified", + "Tests that the current schema qualifier is stripped from function and procedure bodies", + "Tests that string literals containing schema qualifiers are NOT modified" + ] +} diff --git a/testdata/dump/issue_252_function_schema_qualifier/pgdump.sql b/testdata/dump/issue_252_function_schema_qualifier/pgdump.sql new file mode 100644 index 00000000..cc7cfd45 --- /dev/null +++ b/testdata/dump/issue_252_function_schema_qualifier/pgdump.sql @@ -0,0 +1,104 @@ +-- +-- PostgreSQL database dump +-- + +SET statement_timeout = 0; +SET lock_timeout = 0; +SET client_encoding = 'UTF8'; +SET standard_conforming_strings = on; +SET check_function_bodies = false; +SET client_min_messages = warning; +SET row_security = off; + +-- +-- Name: users; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.users ( + id integer NOT NULL, + name text NOT NULL, + email text +); + +-- +-- Name: users_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.users_id_seq + AS integer + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +-- +-- Name: users_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.users_id_seq OWNED BY public.users.id; + +-- +-- Name: users id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.users ALTER COLUMN id SET DEFAULT nextval('public.users_id_seq'::regclass); + +-- +-- Name: users users_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.users + ADD CONSTRAINT users_pkey PRIMARY KEY (id); + +-- +-- Name: get_user_count(); Type: FUNCTION; Schema: public; Owner: - +-- + +CREATE FUNCTION public.get_user_count() RETURNS integer + LANGUAGE plpgsql + AS $$ +BEGIN + RETURN (SELECT count(*)::integer FROM public.users); +END; +$$; + +-- +-- Name: get_user_by_name(text); Type: FUNCTION; Schema: public; Owner: - +-- + +CREATE FUNCTION public.get_user_by_name(p_name text) RETURNS TABLE(id integer, name text, email text) + LANGUAGE plpgsql + AS $$ +BEGIN + RETURN QUERY SELECT u.id, u.name, u.email FROM public.users u WHERE u.name = p_name; +END; +$$; + +-- +-- Name: get_table_info(); Type: FUNCTION; Schema: public; Owner: - +-- + +CREATE FUNCTION public.get_table_info() RETURNS text + LANGUAGE plpgsql + AS $$ +BEGIN + RETURN 'Table: public.users'; +END; +$$; + +-- +-- Name: insert_user(text, text); Type: PROCEDURE; Schema: public; Owner: - +-- + +CREATE PROCEDURE public.insert_user(IN p_name text, IN p_email text) + LANGUAGE plpgsql + AS $$ +BEGIN + INSERT INTO public.users (name, email) VALUES (p_name, p_email); +END; +$$; + +-- +-- PostgreSQL database dump complete +-- diff --git a/testdata/dump/issue_252_function_schema_qualifier/pgschema.sql b/testdata/dump/issue_252_function_schema_qualifier/pgschema.sql new file mode 100644 index 00000000..b83b09ec --- /dev/null +++ b/testdata/dump/issue_252_function_schema_qualifier/pgschema.sql @@ -0,0 +1,78 @@ +-- +-- pgschema database dump +-- + +-- Dumped from database version PostgreSQL 18.0 +-- Dumped by pgschema version 1.7.0 + + +-- +-- Name: users; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS users ( + id SERIAL, + name text NOT NULL, + email text, + CONSTRAINT users_pkey PRIMARY KEY (id) +); + +-- +-- Name: get_table_info(); Type: FUNCTION; Schema: -; Owner: - +-- + +CREATE OR REPLACE FUNCTION get_table_info() +RETURNS text +LANGUAGE plpgsql +VOLATILE +AS $$ +BEGIN + RETURN 'Table: public.users'; +END; +$$; + +-- +-- Name: get_user_by_name(text); Type: FUNCTION; Schema: -; Owner: - +-- + +CREATE OR REPLACE FUNCTION get_user_by_name( + p_name text +) +RETURNS TABLE(id integer, name text, email text) +LANGUAGE plpgsql +VOLATILE +AS $$ +BEGIN + RETURN QUERY SELECT u.id, u.name, u.email FROM users u WHERE u.name = p_name; +END; +$$; + +-- +-- Name: get_user_count(); Type: FUNCTION; Schema: -; Owner: - +-- + +CREATE OR REPLACE FUNCTION get_user_count() +RETURNS integer +LANGUAGE plpgsql +VOLATILE +AS $$ +BEGIN + RETURN (SELECT count(*)::integer FROM users); +END; +$$; + +-- +-- Name: insert_user(text, text); Type: PROCEDURE; Schema: -; Owner: - +-- + +CREATE OR REPLACE PROCEDURE insert_user( + IN p_name text, + IN p_email text +) +LANGUAGE plpgsql +AS $$ +BEGIN + INSERT INTO users (name, email) VALUES (p_name, p_email); +END; +$$; + diff --git a/testdata/dump/issue_252_function_schema_qualifier/raw.sql b/testdata/dump/issue_252_function_schema_qualifier/raw.sql new file mode 100644 index 00000000..a4b1f631 --- /dev/null +++ b/testdata/dump/issue_252_function_schema_qualifier/raw.sql @@ -0,0 +1,55 @@ +-- +-- Test case for GitHub issue #252: Schema qualifiers in function bodies +-- +-- This test reproduces the inconsistency where function bodies retain +-- schema-qualified references (e.g., public.users) while table definitions +-- are dumped without schema qualification. +-- +-- The fix strips the current schema qualifier from function/procedure bodies +-- so the dump output is consistently unqualified. +-- + +CREATE TABLE users ( + id serial PRIMARY KEY, + name text NOT NULL, + email text +); + +-- Function with schema-qualified table reference in body +CREATE OR REPLACE FUNCTION get_user_count() +RETURNS integer +LANGUAGE plpgsql +AS $$ +BEGIN + RETURN (SELECT count(*)::integer FROM public.users); +END; +$$; + +-- Function with multiple schema-qualified references +CREATE OR REPLACE FUNCTION get_user_by_name(p_name text) +RETURNS TABLE(id integer, name text, email text) +LANGUAGE plpgsql +AS $$ +BEGIN + RETURN QUERY SELECT u.id, u.name, u.email FROM public.users u WHERE u.name = p_name; +END; +$$; + +-- Function with string literal containing schema name (should NOT be modified) +CREATE OR REPLACE FUNCTION get_table_info() +RETURNS text +LANGUAGE plpgsql +AS $$ +BEGIN + RETURN 'Table: public.users'; +END; +$$; + +-- Procedure with schema-qualified reference in body +CREATE OR REPLACE PROCEDURE insert_user(p_name text, p_email text) +LANGUAGE plpgsql +AS $$ +BEGIN + INSERT INTO public.users (name, email) VALUES (p_name, p_email); +END; +$$;