Skip to content

[Feature] Add support for Procedures #185

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

Merged
merged 1 commit into from
Dec 11, 2024
Merged

[Feature] Add support for Procedures #185

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
164 changes: 164 additions & 0 deletions internal/migration_acceptance_tests/procedure_cases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
package migration_acceptance_tests

import "github.com/stripe/pg-schema-diff/pkg/diff"

var procedureAcceptanceTestCases = []acceptanceTestCase{
{
name: "No-op",
oldSchemaDDL: []string{
`
CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
BEGIN
RAISE NOTICE 'foobar';
END;
$$ LANGUAGE plpgsql;
`,
},
newSchemaDDL: []string{
`
CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
BEGIN
RAISE NOTICE 'foobar';
END;
$$ LANGUAGE plpgsql;
`,
},

expectEmptyPlan: true,
},
{
name: "Create procedure with no dependencies",
oldSchemaDDL: nil,
newSchemaDDL: []string{
`
CREATE OR REPLACE PROCEDURE some_procedure(val INTEGER) LANGUAGE plpgsql AS $$
BEGIN
RAISE NOTICE 'Val, %', val;
END
$$;
`,
},
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
},
{
name: "Create procedure with dependencies that also must be created",
oldSchemaDDL: nil,
newSchemaDDL: []string{
`
CREATE SEQUENCE user_id_seq;

CREATE TABLE users (
id INTEGER PRIMARY KEY,
name TEXT NOT NULL
);

CREATE OR REPLACE FUNCTION get_name(input_name TEXT) RETURNS TEXT AS $$
SELECT input_name || '_some_fixed_val'
$$ LANGUAGE SQL;

CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO users (id, name) VALUES (NEXTVAL('user_id_seq'), get_name(name));
$$;
`,
},
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
},
{
name: "Alter a procedure to have dependencies that must be created",
oldSchemaDDL: []string{
`
CREATE TABLE users ();
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO users DEFAULT VALUES;
$$;
`,
},
newSchemaDDL: []string{
`
CREATE SEQUENCE user_id_seq;

CREATE TABLE users (
id INTEGER,
name TEXT NOT NULL
);

CREATE OR REPLACE FUNCTION get_name(input_name TEXT) RETURNS TEXT AS $$
SELECT input_name || '_some_fixed_val'
$$ LANGUAGE SQL;

CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO users (id, name) VALUES (NEXTVAL('user_id_seq'), get_name(name));
$$;
`,
},
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
},
{
name: "Drop procedure and its dependencies",
oldSchemaDDL: []string{
`
CREATE SEQUENCE user_id_seq;

CREATE TABLE users (
id INTEGER PRIMARY KEY,
name TEXT NOT NULL
);

CREATE OR REPLACE FUNCTION get_name(input_name TEXT) RETURNS TEXT AS $$
SELECT input_name || '_some_fixed_val'
$$ LANGUAGE SQL;

CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO users (id, name) VALUES (NEXTVAL('user_id_seq'), get_name(name));
$$;
`,
},
newSchemaDDL: []string{
`
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeDeletesData,
diff.MigrationHazardTypeHasUntrackableDependencies,
},
},
{
// This reveals Postgres does not actually track dependencies of procedures outside of creation time.
name: "Drop a procedure's dependencies but not the procedure",
oldSchemaDDL: []string{
`
CREATE TABLE users ();

CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO users DEFAULT VALUES;
$$;
`,
},
newSchemaDDL: []string{
`
CREATE TABLE users();

CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO users DEFAULT VALUES;
$$;

-- Drop the table the procedure depends on. This allows us to actually create a database with this schema.
DROP TABLE users;
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeDeletesData,
},
planOpts: []diff.PlanOpt{
// Skip plan validation because the acceptance test attempts to regenerate the plan after migrating and
// assert it's empty. As part of this plan regeneration, plan validation attempts to create a database with
// just an "Add User" procedure through normal SQL generation, which inherently fails because the users
// table does not exist.
diff.WithDoNotValidatePlan(),
},
},
}

func (suite *acceptanceTestSuite) TestProcedureTestCases() {
suite.runTestCases(procedureAcceptanceTestCases)
}
22 changes: 21 additions & 1 deletion internal/migration_acceptance_tests/schema_cases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ var schemaAcceptanceTests = []acceptanceTestCase{
CREATE INDEX bar_normal_idx ON bar(bar);
CREATE INDEX bar_another_normal_id ON bar(bar, fizz);
CREATE UNIQUE INDEX bar_unique_idx on bar(foo, buzz);

CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
BEGIN
RAISE NOTICE 'foobar';
END;
$$ LANGUAGE plpgsql;
`,
},
newSchemaDDL: []string{
Expand Down Expand Up @@ -156,12 +162,18 @@ var schemaAcceptanceTests = []acceptanceTestCase{
CREATE INDEX bar_normal_idx ON bar(bar);
CREATE INDEX bar_another_normal_id ON bar(bar, fizz);
CREATE UNIQUE INDEX bar_unique_idx on bar(foo, buzz);

CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
BEGIN
RAISE NOTICE 'foobar';
END;
$$ LANGUAGE plpgsql;
`,
},
expectEmptyPlan: true,
},
{
name: "Add schema, drop schema, Add enum, Drop enum, Drop table, Add Table, Drop Seq, Add Seq, Drop Funcs, Add Funcs, Drop Triggers, Add Triggers, Create Extension, Drop Extension, Create Index Using Extension, Add policies, Drop policies",
name: "Add/drop all objects",
roles: []string{"role_1"},
oldSchemaDDL: []string{
`
Expand Down Expand Up @@ -219,6 +231,10 @@ var schemaAcceptanceTests = []acceptanceTestCase{
CREATE INDEX foobar_normal_idx ON foobar USING hash (fizz);
CREATE UNIQUE INDEX foobar_unique_idx ON foobar(foo, fizz DESC);

CREATE OR REPLACE PROCEDURE add_foobar(name TEXT) LANGUAGE SQL AS $$
INSERT INTO foobar DEFAULT VALUES
$$;

CREATE POLICY foobar_foo_policy ON foobar FOR SELECT TO PUBLIC USING (foo = current_user);

CREATE TRIGGER "some trigger"
Expand Down Expand Up @@ -303,6 +319,10 @@ var schemaAcceptanceTests = []acceptanceTestCase{

CREATE POLICY "New_table_foo_policy" ON "New_table" FOR DELETE TO PUBLIC USING (version > 0);

CREATE OR REPLACE PROCEDURE "new new table"(name TEXT) LANGUAGE SQL AS $$
INSERT INTO "New_table" (id, version) VALUES (NEXTVAL('schema_3.new_foobar_sequence'), schema_3."new add"(LENGTH(name), 1))
$$;

CREATE TRIGGER "some trigger"
BEFORE UPDATE ON "New_table"
FOR EACH ROW
Expand Down
48 changes: 28 additions & 20 deletions internal/queries/queries.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ WHERE

-- name: GetTables :many
SELECT
c.oid AS oid,
c.oid,
c.relname::TEXT AS table_name,
table_namespace.nspname::TEXT AS table_schema_name,
c.relreplident::TEXT AS replica_identity,
Expand All @@ -40,7 +40,7 @@ INNER JOIN
ON c.relnamespace = table_namespace.oid
LEFT JOIN
pg_catalog.pg_inherits AS table_inherits
ON table_inherits.inhrelid = c.oid
ON c.oid = table_inherits.inhrelid
LEFT JOIN
pg_catalog.pg_class AS parent_c
ON table_inherits.inhparent = parent_c.oid
Expand Down Expand Up @@ -98,16 +98,16 @@ SELECT
FROM pg_catalog.pg_attribute AS a
LEFT JOIN
pg_catalog.pg_attrdef AS d
ON (d.adrelid = a.attrelid AND d.adnum = a.attnum)
LEFT JOIN pg_catalog.pg_collation AS coll ON coll.oid = a.attcollation
ON (a.attrelid = d.adrelid AND a.attnum = d.adnum)
LEFT JOIN pg_catalog.pg_collation AS coll ON a.attcollation = coll.oid
LEFT JOIN
pg_catalog.pg_namespace AS collation_namespace
ON collation_namespace.oid = coll.collnamespace
ON coll.collnamespace = collation_namespace.oid
LEFT JOIN
identity_col_seq
ON
identity_col_seq.owner_relid = a.attrelid
AND identity_col_seq.owner_attnum = a.attnum
a.attrelid = identity_col_seq.owner_relid
AND a.attnum = identity_col_seq.owner_attnum
WHERE
a.attrelid = $1
AND a.attnum > 0
Expand All @@ -116,7 +116,7 @@ ORDER BY a.attnum;

-- name: GetIndexes :many
SELECT
c.oid AS oid,
c.oid,
c.relname::TEXT AS index_name,
table_c.relname::TEXT AS table_name,
table_namespace.nspname::TEXT AS table_schema_name,
Expand All @@ -132,21 +132,25 @@ SELECT
COALESCE(parent_c.relname, '')::TEXT AS parent_index_name,
COALESCE(parent_namespace.nspname, '')::TEXT AS parent_index_schema_name,
(
SELECT ARRAY_AGG(att.attname ORDER BY indkey_ord.ord)
SELECT
ARRAY_AGG(
att.attname
ORDER BY indkey_ord.ord
)
FROM UNNEST(i.indkey) WITH ORDINALITY AS indkey_ord (attnum, ord)
INNER JOIN
pg_catalog.pg_attribute AS att
ON att.attrelid = table_c.oid AND att.attnum = indkey_ord.attnum
ON att.attrelid = table_c.oid AND indkey_ord.attnum = att.attnum
)::TEXT [] AS column_names,
COALESCE(con.conislocal, false) AS constraint_is_local
FROM pg_catalog.pg_class AS c
INNER JOIN pg_catalog.pg_index AS i ON (i.indexrelid = c.oid)
INNER JOIN pg_catalog.pg_class AS table_c ON (table_c.oid = i.indrelid)
INNER JOIN pg_catalog.pg_index AS i ON (c.oid = i.indexrelid)
INNER JOIN pg_catalog.pg_class AS table_c ON (i.indrelid = table_c.oid)
INNER JOIN pg_catalog.pg_namespace AS table_namespace
ON table_c.relnamespace = table_namespace.oid
LEFT JOIN
pg_catalog.pg_constraint AS con
ON (con.conindid = c.oid AND con.contype IN ('p', 'u', null))
ON (c.oid = con.conindid AND con.contype IN ('p', 'u', null))
LEFT JOIN
pg_catalog.pg_inherits AS idx_inherits
ON (c.oid = idx_inherits.inhrelid)
Expand Down Expand Up @@ -222,7 +226,7 @@ WHERE
AND pg_constraint.contype = 'f'
AND pg_constraint.conislocal;

-- name: GetFunctions :many
-- name: GetProcs :many
SELECT
pg_proc.oid,
pg_proc.proname::TEXT AS func_name,
Expand All @@ -238,12 +242,12 @@ INNER JOIN
ON pg_proc.pronamespace = proc_namespace.oid
INNER JOIN
pg_catalog.pg_language AS proc_lang
ON proc_lang.oid = pg_proc.prolang
ON pg_proc.prolang = proc_lang.oid
WHERE
proc_namespace.nspname NOT IN ('pg_catalog', 'information_schema')
AND proc_namespace.nspname !~ '^pg_toast'
AND proc_namespace.nspname !~ '^pg_temp'
AND pg_proc.prokind = 'f'
AND pg_proc.prokind = $1
-- Exclude functions belonging to extensions
AND NOT EXISTS (
SELECT depend.objid
Expand Down Expand Up @@ -358,7 +362,7 @@ SELECT
FROM pg_catalog.pg_namespace AS extension_namespace
INNER JOIN
pg_catalog.pg_extension AS ext
ON ext.extnamespace = extension_namespace.oid
ON extension_namespace.oid = ext.extnamespace
WHERE
extension_namespace.nspname NOT IN ('pg_catalog', 'information_schema')
AND extension_namespace.nspname !~ '^pg_toast'
Expand All @@ -370,14 +374,18 @@ SELECT
pg_type.typname::TEXT AS enum_name,
type_namespace.nspname::TEXT AS enum_schema_name,
(
SELECT ARRAY_AGG(pg_enum.enumlabel ORDER BY pg_enum.enumsortorder)
SELECT
ARRAY_AGG(
pg_enum.enumlabel
ORDER BY pg_enum.enumsortorder
)
FROM pg_catalog.pg_enum
WHERE pg_enum.enumtypid = pg_type.oid
)::TEXT [] AS enum_labels
FROM pg_catalog.pg_type AS pg_type
INNER JOIN
pg_catalog.pg_namespace AS type_namespace
ON type_namespace.oid = pg_type.typnamespace
ON pg_type.typnamespace = type_namespace.oid
WHERE
pg_type.typtype = 'e'
AND type_namespace.nspname NOT IN ('pg_catalog', 'information_schema')
Expand Down Expand Up @@ -414,7 +422,7 @@ SELECT
table_namespace.nspname::TEXT AS owning_table_schema_name,
pol.polpermissive AS is_permissive,
(
SELECT ARRAY_AGG(rolname)
SELECT ARRAY_AGG(roles.rolname)
FROM roles
WHERE roles.oid = ANY(pol.polroles)
)::TEXT [] AS applies_to,
Expand Down
Loading
Loading