fix: create SQL functions after the later-batch functions they call (#596)

internal/diff/diff.go

@@ -2008,6 +2008,8 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto
 				functionsWithoutViewDeps = append(functionsWithoutViewDeps, fn)
 			}
 		}
+		// A SQL function calling a view-dependent function waits for it too.
+		functionsWithoutViewDeps, functionsWithViewDeps = holdBackCallers(functionsWithoutViewDeps, functionsWithViewDeps)
 	}
 
 	// Functions whose return/parameter type references a view being recreated
@@ -2069,6 +2071,9 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto
 				functionsWithoutTableDeps = append(functionsWithoutTableDeps, fn)
 			}
 		}
+		// A SQL function calling a function from a later batch is validated against
+		// it at creation, so it moves to that batch even if it touches no new table.
+		functionsWithoutTableDeps, functionsWithTableDeps, functionsAfterAllTables = holdBackCallersAcrossBatches(functionsWithoutTableDeps, functionsWithTableDeps, functionsAfterAllTables)
 	}
 
 	// Create functions WITHOUT view dependencies AND WITHOUT table dependencies
@@ -2918,12 +2923,37 @@ func splitFunctionsCallingAggregates(functions []*ir.Function, aggregates []*ir.
 		}
 	}
 	// Transitive closure: a function that calls a held-back function waits too.
+	return holdBackCallers(now, later)
+}
+
+// holdBackCallersAcrossBatches applies holdBackCallers to three consecutive function
+// batches until none changes. A single round is not enough: a function that joins a
+// later batch in one step can strand a caller that an earlier step already examined.
+func holdBackCallersAcrossBatches(first, second, third []*ir.Function) ([]*ir.Function, []*ir.Function, []*ir.Function) {
+	for {
+		secondLen, thirdLen := len(second), len(third)
+		second, third = holdBackCallers(second, third)
+		first, third = holdBackCallers(first, third)
+		first, second = holdBackCallers(first, second)
+		// Functions only ever move to a later batch, so unchanged sizes mean a fixpoint.
+		if len(second) == secondLen && len(third) == thirdLen {
+			return first, second, third
+		}
+	}
+}
+
+// holdBackCallers moves every SQL-language function in now that calls a function
+// in later over to later, transitively. A SQL-language body is validated when the
+// function is created, so a caller cannot be created in an earlier batch than its
+// callee; topologicallySortFunctions only orders functions within one batch.
+// Order within now is preserved.
+func holdBackCallers(now, later []*ir.Function) ([]*ir.Function, []*ir.Function) {
 	for changed := len(later) > 0; changed; {
 		changed = false
 		lateLookup := buildFunctionLookup(later)
 		var still []*ir.Function
 		for _, fn := range now {
-			if calls(fn, lateLookup) {
+			if strings.EqualFold(fn.Language, "sql") && referencesNewFunction(fn.Definition, fn.Schema, lateLookup) {
 				later = append(later, fn)
 				changed = true
 			} else {

internal/diff/function_batches_test.go

@@ -0,0 +1,51 @@
+package diff
+
+import (
+	"testing"
+
+	"github.com/pgplex/pgschema/ir"
+)
+
+func TestHoldBackCallersAcrossBatches(t *testing.T) {
+	fn := func(name, language, body string) *ir.Function {
+		return &ir.Function{Schema: "public", Name: name, Language: language, Definition: body}
+	}
+	names := func(fns []*ir.Function) []string {
+		var out []string
+		for _, f := range fns {
+			out = append(out, f.Name)
+		}
+		return out
+	}
+
+	// caller (second batch) -> helper (first batch) -> last (third batch): helper only
+	// joins the third batch after caller was first examined, so caller needs another round.
+	helper := fn("helper", "sql", "SELECT last()")
+	independent := fn("independent", "sql", "SELECT 1")
+	dynamic := fn("dynamic", "plpgsql", "BEGIN RETURN last(); END")
+	caller := fn("caller", "sql", "SELECT helper() FROM t")
+	last := fn("last", "sql", "SELECT count(*) FROM t2")
+
+	first, second, third := holdBackCallersAcrossBatches(
+		[]*ir.Function{helper, independent, dynamic},
+		[]*ir.Function{caller},
+		[]*ir.Function{last},
+	)
+
+	assertNames := func(label string, got []*ir.Function, want ...string) {
+		t.Helper()
+		g := names(got)
+		if len(g) != len(want) {
+			t.Fatalf("%s: got %v, want %v", label, g, want)
+		}
+		for i := range want {
+			if g[i] != want[i] {
+				t.Fatalf("%s: got %v, want %v", label, g, want)
+			}
+		}
+	}
+	// plpgsql resolves calls at run time and is never held back.
+	assertNames("first", first, "independent", "dynamic")
+	assertNames("second", second)
+	assertNames("third", third, "last", "helper", "caller")
+}

testdata/diff/dependency/issue_530_function_table_function_chain/diff.sql

@@ -37,3 +37,11 @@ STABLE
 AS $$
     SELECT x.flag FROM x WHERE x.id = id;
 $$;
+
+CREATE OR REPLACE FUNCTION first_is_flagged()
+RETURNS boolean
+LANGUAGE sql
+STABLE
+AS $$
+    SELECT x_is_flagged(1);
+$$;

testdata/diff/dependency/issue_530_function_table_function_chain/new.sql

@@ -23,3 +23,10 @@ BEGIN
     RETURN row_x.flag;
 END;
 $$;
+
+-- SQL function that never touches table x itself but calls x_is_flagged, which is
+-- created after x. Its body is validated at creation, so it must follow x_is_flagged.
+CREATE FUNCTION public.first_is_flagged()
+RETURNS boolean LANGUAGE sql STABLE AS $$
+    SELECT x_is_flagged(1);
+$$;

testdata/diff/dependency/issue_530_function_table_function_chain/plan.json

@@ -31,6 +31,12 @@
           "type": "function",
           "operation": "create",
           "path": "public.x_is_flagged"
+        },
+        {
+          "sql": "CREATE OR REPLACE FUNCTION first_is_flagged()\nRETURNS boolean\nLANGUAGE sql\nSTABLE\nAS $$\n    SELECT x_is_flagged(1);\n$$;",
+          "type": "function",
+          "operation": "create",
+          "path": "public.first_is_flagged"
         }
       ]
     }

testdata/diff/dependency/issue_530_function_table_function_chain/plan.sql

@@ -37,3 +37,11 @@ STABLE
 AS $$
     SELECT x.flag FROM x WHERE x.id = id;
 $$;
+
+CREATE OR REPLACE FUNCTION first_is_flagged()
+RETURNS boolean
+LANGUAGE sql
+STABLE
+AS $$
+    SELECT x_is_flagged(1);
+$$;

testdata/diff/dependency/issue_530_function_table_function_chain/plan.txt

@@ -1,10 +1,11 @@
-Plan: 4 to add.
+Plan: 5 to add.
 
 Summary by type:
-  functions: 3 to add
+  functions: 4 to add
   tables: 1 to add
 
 Functions:
+  + first_is_flagged
   + random_id
   + x_check
   + x_is_flagged
@@ -54,3 +55,11 @@ STABLE
 AS $$
     SELECT x.flag FROM x WHERE x.id = id;
 $$;
+
+CREATE OR REPLACE FUNCTION first_is_flagged()
+RETURNS boolean
+LANGUAGE sql
+STABLE
+AS $$
+    SELECT x_is_flagged(1);
+$$;

testdata/diff/dependency/issue_580_aggregate_and_sql_body_to_view/diff.sql

@@ -95,6 +95,13 @@ VOLATILE
 AS $$ SELECT count(*) FROM v
 $$;
 
+CREATE OR REPLACE FUNCTION count_v_twice()
+RETURNS bigint
+LANGUAGE sql
+VOLATILE
+AS $$ SELECT 2 * count_v()
+$$;
+
 CREATE OR REPLACE FUNCTION ob_sfunc(
     state integer,
     r "Order By V"

testdata/diff/dependency/issue_580_aggregate_and_sql_body_to_view/new.sql

@@ -153,3 +153,11 @@ CREATE FUNCTION count_sub_v()
 RETURNS bigint
 LANGUAGE sql
 AS $$ SELECT count(*) FROM (SELECT 1) AS s, v $$;
+
+-- SQL-language function that never mentions the view but calls count_v, which
+-- is held for the view batch: its body is validated at creation, so it must
+-- follow count_v (issue #596).
+CREATE FUNCTION count_v_twice()
+RETURNS bigint
+LANGUAGE sql
+AS $$ SELECT 2 * count_v() $$;

testdata/diff/dependency/issue_580_aggregate_and_sql_body_to_view/plan.json

@@ -98,6 +98,12 @@
           "operation": "create",
           "path": "public.count_v"
         },
+        {
+          "sql": "CREATE OR REPLACE FUNCTION count_v_twice()\nRETURNS bigint\nLANGUAGE sql\nVOLATILE\nAS $$ SELECT 2 * count_v()\n$$;",
+          "type": "function",
+          "operation": "create",
+          "path": "public.count_v_twice"
+        },
         {
           "sql": "CREATE OR REPLACE FUNCTION ob_sfunc(\n    state integer,\n    r \"Order By V\"\n)\nRETURNS integer\nLANGUAGE sql\nIMMUTABLE\nAS $$ SELECT state + r.id\n$$;",
           "type": "function",

testdata/diff/dependency/issue_580_aggregate_and_sql_body_to_view/plan.sql

@@ -95,6 +95,13 @@ VOLATILE
 AS $$ SELECT count(*) FROM v
 $$;
 
+CREATE OR REPLACE FUNCTION count_v_twice()
+RETURNS bigint
+LANGUAGE sql
+VOLATILE
+AS $$ SELECT 2 * count_v()
+$$;
+
 CREATE OR REPLACE FUNCTION ob_sfunc(
     state integer,
     r "Order By V"

testdata/diff/dependency/issue_580_aggregate_and_sql_body_to_view/plan.txt

@@ -1,7 +1,7 @@
-Plan: 25 to add.
+Plan: 26 to add.
 
 Summary by type:
-  functions: 15 to add
+  functions: 16 to add
   aggregates: 5 to add
   tables: 1 to add
   views: 4 to add
@@ -16,6 +16,7 @@ Functions:
   + count_sub_v
   + count_unnest_v
   + count_v
+  + count_v_twice
   + get_total
   + ob_sfunc
   + total_v
@@ -140,6 +141,13 @@ VOLATILE
 AS $$ SELECT count(*) FROM v
 $$;
 
+CREATE OR REPLACE FUNCTION count_v_twice()
+RETURNS bigint
+LANGUAGE sql
+VOLATILE
+AS $$ SELECT 2 * count_v()
+$$;
+
 CREATE OR REPLACE FUNCTION ob_sfunc(
     state integer,
     r "Order By V"