Skip to content

Commit 72e3df5

Browse files
bplunkett-stripebplunkett-stripe
authored
Add proc support (#185)
1 parent 21fab6a commit 72e3df5

9 files changed

+655
-231
lines changed

‎internal/migration_acceptance_tests/procedure_cases_test.go

+164
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
package migration_acceptance_tests
2+
3+
import "github.com/stripe/pg-schema-diff/pkg/diff"
4+
5+
var procedureAcceptanceTestCases = []acceptanceTestCase{
6+
{
7+
name: "No-op",
8+
oldSchemaDDL: []string{
9+
`
10+
CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
11+
BEGIN
12+
RAISE NOTICE 'foobar';
13+
END;
14+
$$ LANGUAGE plpgsql;
15+
`,
16+
},
17+
newSchemaDDL: []string{
18+
`
19+
CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
20+
BEGIN
21+
RAISE NOTICE 'foobar';
22+
END;
23+
$$ LANGUAGE plpgsql;
24+
`,
25+
},
26+
27+
expectEmptyPlan: true,
28+
},
29+
{
30+
name: "Create procedure with no dependencies",
31+
oldSchemaDDL: nil,
32+
newSchemaDDL: []string{
33+
`
34+
CREATE OR REPLACE PROCEDURE some_procedure(val INTEGER) LANGUAGE plpgsql AS $$
35+
BEGIN
36+
RAISE NOTICE 'Val, %', val;
37+
END
38+
$$;
39+
`,
40+
},
41+
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
42+
},
43+
{
44+
name: "Create procedure with dependencies that also must be created",
45+
oldSchemaDDL: nil,
46+
newSchemaDDL: []string{
47+
`
48+
CREATE SEQUENCE user_id_seq;
49+
50+
CREATE TABLE users (
51+
id INTEGER PRIMARY KEY,
52+
name TEXT NOT NULL
53+
);
54+
55+
CREATE OR REPLACE FUNCTION get_name(input_name TEXT) RETURNS TEXT AS $$
56+
SELECT input_name || '_some_fixed_val'
57+
$$ LANGUAGE SQL;
58+
59+
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
60+
INSERT INTO users (id, name) VALUES (NEXTVAL('user_id_seq'), get_name(name));
61+
$$;
62+
`,
63+
},
64+
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
65+
},
66+
{
67+
name: "Alter a procedure to have dependencies that must be created",
68+
oldSchemaDDL: []string{
69+
`
70+
CREATE TABLE users ();
71+
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
72+
INSERT INTO users DEFAULT VALUES;
73+
$$;
74+
`,
75+
},
76+
newSchemaDDL: []string{
77+
`
78+
CREATE SEQUENCE user_id_seq;
79+
80+
CREATE TABLE users (
81+
id INTEGER,
82+
name TEXT NOT NULL
83+
);
84+
85+
CREATE OR REPLACE FUNCTION get_name(input_name TEXT) RETURNS TEXT AS $$
86+
SELECT input_name || '_some_fixed_val'
87+
$$ LANGUAGE SQL;
88+
89+
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
90+
INSERT INTO users (id, name) VALUES (NEXTVAL('user_id_seq'), get_name(name));
91+
$$;
92+
`,
93+
},
94+
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
95+
},
96+
{
97+
name: "Drop procedure and its dependencies",
98+
oldSchemaDDL: []string{
99+
`
100+
CREATE SEQUENCE user_id_seq;
101+
102+
CREATE TABLE users (
103+
id INTEGER PRIMARY KEY,
104+
name TEXT NOT NULL
105+
);
106+
107+
CREATE OR REPLACE FUNCTION get_name(input_name TEXT) RETURNS TEXT AS $$
108+
SELECT input_name || '_some_fixed_val'
109+
$$ LANGUAGE SQL;
110+
111+
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
112+
INSERT INTO users (id, name) VALUES (NEXTVAL('user_id_seq'), get_name(name));
113+
$$;
114+
`,
115+
},
116+
newSchemaDDL: []string{
117+
`
118+
`,
119+
},
120+
expectedHazardTypes: []diff.MigrationHazardType{
121+
diff.MigrationHazardTypeDeletesData,
122+
diff.MigrationHazardTypeHasUntrackableDependencies,
123+
},
124+
},
125+
{
126+
// This reveals Postgres does not actually track dependencies of procedures outside of creation time.
127+
name: "Drop a procedure's dependencies but not the procedure",
128+
oldSchemaDDL: []string{
129+
`
130+
CREATE TABLE users ();
131+
132+
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
133+
INSERT INTO users DEFAULT VALUES;
134+
$$;
135+
`,
136+
},
137+
newSchemaDDL: []string{
138+
`
139+
CREATE TABLE users();
140+
141+
CREATE OR REPLACE PROCEDURE "Add User"(name TEXT) LANGUAGE SQL AS $$
142+
INSERT INTO users DEFAULT VALUES;
143+
$$;
144+
145+
-- Drop the table the procedure depends on. This allows us to actually create a database with this schema.
146+
DROP TABLE users;
147+
`,
148+
},
149+
expectedHazardTypes: []diff.MigrationHazardType{
150+
diff.MigrationHazardTypeDeletesData,
151+
},
152+
planOpts: []diff.PlanOpt{
153+
// Skip plan validation because the acceptance test attempts to regenerate the plan after migrating and
154+
// assert it's empty. As part of this plan regeneration, plan validation attempts to create a database with
155+
// just an "Add User" procedure through normal SQL generation, which inherently fails because the users
156+
// table does not exist.
157+
diff.WithDoNotValidatePlan(),
158+
},
159+
},
160+
}
161+
162+
func (suite *acceptanceTestSuite) TestProcedureTestCases() {
163+
suite.runTestCases(procedureAcceptanceTestCases)
164+
}

‎internal/migration_acceptance_tests/schema_cases_test.go

+21-1
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ var schemaAcceptanceTests = []acceptanceTestCase{
8181
CREATE INDEX bar_normal_idx ON bar(bar);
8282
CREATE INDEX bar_another_normal_id ON bar(bar, fizz);
8383
CREATE UNIQUE INDEX bar_unique_idx on bar(foo, buzz);
84+
85+
CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
86+
BEGIN
87+
RAISE NOTICE 'foobar';
88+
END;
89+
$$ LANGUAGE plpgsql;
8490
`,
8591
},
8692
newSchemaDDL: []string{
@@ -156,12 +162,18 @@ var schemaAcceptanceTests = []acceptanceTestCase{
156162
CREATE INDEX bar_normal_idx ON bar(bar);
157163
CREATE INDEX bar_another_normal_id ON bar(bar, fizz);
158164
CREATE UNIQUE INDEX bar_unique_idx on bar(foo, buzz);
165+
166+
CREATE OR REPLACE PROCEDURE some_procedure(i integer) AS $$
167+
BEGIN
168+
RAISE NOTICE 'foobar';
169+
END;
170+
$$ LANGUAGE plpgsql;
159171
`,
160172
},
161173
expectEmptyPlan: true,
162174
},
163175
{
164-
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",
176+
name: "Add/drop all objects",
165177
roles: []string{"role_1"},
166178
oldSchemaDDL: []string{
167179
`
@@ -219,6 +231,10 @@ var schemaAcceptanceTests = []acceptanceTestCase{
219231
CREATE INDEX foobar_normal_idx ON foobar USING hash (fizz);
220232
CREATE UNIQUE INDEX foobar_unique_idx ON foobar(foo, fizz DESC);
221233
234+
CREATE OR REPLACE PROCEDURE add_foobar(name TEXT) LANGUAGE SQL AS $$
235+
INSERT INTO foobar DEFAULT VALUES
236+
$$;
237+
222238
CREATE POLICY foobar_foo_policy ON foobar FOR SELECT TO PUBLIC USING (foo = current_user);
223239
224240
CREATE TRIGGER "some trigger"
@@ -303,6 +319,10 @@ var schemaAcceptanceTests = []acceptanceTestCase{
303319
304320
CREATE POLICY "New_table_foo_policy" ON "New_table" FOR DELETE TO PUBLIC USING (version > 0);
305321
322+
CREATE OR REPLACE PROCEDURE "new new table"(name TEXT) LANGUAGE SQL AS $$
323+
INSERT INTO "New_table" (id, version) VALUES (NEXTVAL('schema_3.new_foobar_sequence'), schema_3."new add"(LENGTH(name), 1))
324+
$$;
325+
306326
CREATE TRIGGER "some trigger"
307327
BEFORE UPDATE ON "New_table"
308328
FOR EACH ROW

‎internal/queries/queries.sql

+28-20
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ WHERE
1717

1818
-- name: GetTables :many
1919
SELECT
20-
c.oid AS oid,
20+
c.oid,
2121
c.relname::TEXT AS table_name,
2222
table_namespace.nspname::TEXT AS table_schema_name,
2323
c.relreplident::TEXT AS replica_identity,
@@ -40,7 +40,7 @@ INNER JOIN
4040
ON c.relnamespace = table_namespace.oid
4141
LEFT JOIN
4242
pg_catalog.pg_inherits AS table_inherits
43-
ON table_inherits.inhrelid = c.oid
43+
ON c.oid = table_inherits.inhrelid
4444
LEFT JOIN
4545
pg_catalog.pg_class AS parent_c
4646
ON table_inherits.inhparent = parent_c.oid
@@ -98,16 +98,16 @@ SELECT
9898
FROM pg_catalog.pg_attribute AS a
9999
LEFT JOIN
100100
pg_catalog.pg_attrdef AS d
101-
ON (d.adrelid = a.attrelid AND d.adnum = a.attnum)
102-
LEFT JOIN pg_catalog.pg_collation AS coll ON coll.oid = a.attcollation
101+
ON (a.attrelid = d.adrelid AND a.attnum = d.adnum)
102+
LEFT JOIN pg_catalog.pg_collation AS coll ON a.attcollation = coll.oid
103103
LEFT JOIN
104104
pg_catalog.pg_namespace AS collation_namespace
105-
ON collation_namespace.oid = coll.collnamespace
105+
ON coll.collnamespace = collation_namespace.oid
106106
LEFT JOIN
107107
identity_col_seq
108108
ON
109-
identity_col_seq.owner_relid = a.attrelid
110-
AND identity_col_seq.owner_attnum = a.attnum
109+
a.attrelid = identity_col_seq.owner_relid
110+
AND a.attnum = identity_col_seq.owner_attnum
111111
WHERE
112112
a.attrelid = $1
113113
AND a.attnum > 0
@@ -116,7 +116,7 @@ ORDER BY a.attnum;
116116

117117
-- name: GetIndexes :many
118118
SELECT
119-
c.oid AS oid,
119+
c.oid,
120120
c.relname::TEXT AS index_name,
121121
table_c.relname::TEXT AS table_name,
122122
table_namespace.nspname::TEXT AS table_schema_name,
@@ -132,21 +132,25 @@ SELECT
132132
COALESCE(parent_c.relname, '')::TEXT AS parent_index_name,
133133
COALESCE(parent_namespace.nspname, '')::TEXT AS parent_index_schema_name,
134134
(
135-
SELECT ARRAY_AGG(att.attname ORDER BY indkey_ord.ord)
135+
SELECT
136+
ARRAY_AGG(
137+
att.attname
138+
ORDER BY indkey_ord.ord
139+
)
136140
FROM UNNEST(i.indkey) WITH ORDINALITY AS indkey_ord (attnum, ord)
137141
INNER JOIN
138142
pg_catalog.pg_attribute AS att
139-
ON att.attrelid = table_c.oid AND att.attnum = indkey_ord.attnum
143+
ON att.attrelid = table_c.oid AND indkey_ord.attnum = att.attnum
140144
)::TEXT [] AS column_names,
141145
COALESCE(con.conislocal, false) AS constraint_is_local
142146
FROM pg_catalog.pg_class AS c
143-
INNER JOIN pg_catalog.pg_index AS i ON (i.indexrelid = c.oid)
144-
INNER JOIN pg_catalog.pg_class AS table_c ON (table_c.oid = i.indrelid)
147+
INNER JOIN pg_catalog.pg_index AS i ON (c.oid = i.indexrelid)
148+
INNER JOIN pg_catalog.pg_class AS table_c ON (i.indrelid = table_c.oid)
145149
INNER JOIN pg_catalog.pg_namespace AS table_namespace
146150
ON table_c.relnamespace = table_namespace.oid
147151
LEFT JOIN
148152
pg_catalog.pg_constraint AS con
149-
ON (con.conindid = c.oid AND con.contype IN ('p', 'u', null))
153+
ON (c.oid = con.conindid AND con.contype IN ('p', 'u', null))
150154
LEFT JOIN
151155
pg_catalog.pg_inherits AS idx_inherits
152156
ON (c.oid = idx_inherits.inhrelid)
@@ -222,7 +226,7 @@ WHERE
222226
AND pg_constraint.contype = 'f'
223227
AND pg_constraint.conislocal;
224228

225-
-- name: GetFunctions :many
229+
-- name: GetProcs :many
226230
SELECT
227231
pg_proc.oid,
228232
pg_proc.proname::TEXT AS func_name,
@@ -238,12 +242,12 @@ INNER JOIN
238242
ON pg_proc.pronamespace = proc_namespace.oid
239243
INNER JOIN
240244
pg_catalog.pg_language AS proc_lang
241-
ON proc_lang.oid = pg_proc.prolang
245+
ON pg_proc.prolang = proc_lang.oid
242246
WHERE
243247
proc_namespace.nspname NOT IN ('pg_catalog', 'information_schema')
244248
AND proc_namespace.nspname !~ '^pg_toast'
245249
AND proc_namespace.nspname !~ '^pg_temp'
246-
AND pg_proc.prokind = 'f'
250+
AND pg_proc.prokind = $1
247251
-- Exclude functions belonging to extensions
248252
AND NOT EXISTS (
249253
SELECT depend.objid
@@ -358,7 +362,7 @@ SELECT
358362
FROM pg_catalog.pg_namespace AS extension_namespace
359363
INNER JOIN
360364
pg_catalog.pg_extension AS ext
361-
ON ext.extnamespace = extension_namespace.oid
365+
ON extension_namespace.oid = ext.extnamespace
362366
WHERE
363367
extension_namespace.nspname NOT IN ('pg_catalog', 'information_schema')
364368
AND extension_namespace.nspname !~ '^pg_toast'
@@ -370,14 +374,18 @@ SELECT
370374
pg_type.typname::TEXT AS enum_name,
371375
type_namespace.nspname::TEXT AS enum_schema_name,
372376
(
373-
SELECT ARRAY_AGG(pg_enum.enumlabel ORDER BY pg_enum.enumsortorder)
377+
SELECT
378+
ARRAY_AGG(
379+
pg_enum.enumlabel
380+
ORDER BY pg_enum.enumsortorder
381+
)
374382
FROM pg_catalog.pg_enum
375383
WHERE pg_enum.enumtypid = pg_type.oid
376384
)::TEXT [] AS enum_labels
377385
FROM pg_catalog.pg_type AS pg_type
378386
INNER JOIN
379387
pg_catalog.pg_namespace AS type_namespace
380-
ON type_namespace.oid = pg_type.typnamespace
388+
ON pg_type.typnamespace = type_namespace.oid
381389
WHERE
382390
pg_type.typtype = 'e'
383391
AND type_namespace.nspname NOT IN ('pg_catalog', 'information_schema')
@@ -414,7 +422,7 @@ SELECT
414422
table_namespace.nspname::TEXT AS owning_table_schema_name,
415423
pol.polpermissive AS is_permissive,
416424
(
417-
SELECT ARRAY_AGG(rolname)
425+
SELECT ARRAY_AGG(roles.rolname)
418426
FROM roles
419427
WHERE roles.oid = ANY(pol.polroles)
420428
)::TEXT [] AS applies_to,

0 commit comments

Comments
 (0)