Skip to content

fix: function dump default #126

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
tianzhou merged 1 commit into from
Oct 27, 2025
Merged

fix: function dump default #126

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
7 changes: 7 additions & 0 deletions cmd/dump/dump_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ func TestDumpCommand_Issue83ExplicitConstraintName(t *testing.T) {
runExactMatchTest(t, "issue_83_explicit_constraint_name")
}

func TestDumpCommand_Issue125FunctionDefault(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
runExactMatchTest(t, "issue_125_function_default")
}

func runExactMatchTest(t *testing.T, testDataDir string) {
runExactMatchTestWithContext(t, context.Background(), testDataDir)
}
Expand Down
74 changes: 3 additions & 71 deletions ir/inspector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,14 +907,9 @@ func (i *Inspector) buildFunctions(ctx context.Context, schema *IR, targetSchema
// Handle security definer
isSecurityDefiner := fn.IsSecurityDefiner

// Parse parameters from system catalog arrays (preferred method)
var parameters []*Parameter
if arrayParams := i.parseParametersFromProcArrays(fn.Proargmodes, fn.Proargnames, fn.Proallargtypes); arrayParams != nil {
parameters = arrayParams
} else {
// Fall back to parsing from signature for simple functions without modes
parameters = i.parseParametersFromSignature(signature)
}
// Parse parameters from the complete signature provided by pg_get_function_arguments()
// This signature includes all parameter information including modes, names, types, and defaults
parameters := i.parseParametersFromSignature(signature)

function := &Function{
Schema: schemaName,
Expand Down Expand Up @@ -1066,69 +1061,6 @@ func (i *Inspector) parseParametersFromSignature(signature string) []*Parameter
return parameters
}

// parseParametersFromProcArrays parses function parameters from PostgreSQL system catalog arrays
// proargmodes: parameter modes (i=IN, o=OUT, b=INOUT, v=VARIADIC, t=TABLE)
// proargnames: parameter names
// proallargtypes: parameter type OIDs (as text)
func (i *Inspector) parseParametersFromProcArrays(proargmodes []string, proargnames []string, proallargtypes []string) []*Parameter {
// If no modes are specified, all parameters are IN parameters
if len(proargmodes) == 0 {
// Fall back to parsing from signature for simple functions
return nil
}

var parameters []*Parameter
position := 1

for idx, modeStr := range proargmodes {

// Convert PostgreSQL mode characters to our mode strings
var mode string
switch modeStr {
case "i":
mode = "IN"
case "o":
mode = "OUT"
case "b":
mode = "INOUT"
case "v":
mode = "VARIADIC"
case "t":
mode = "TABLE"
default:
mode = "IN" // Default to IN for unknown modes
}

parameter := &Parameter{
Mode: mode,
Position: position,
}

// Extract parameter name if available
if idx < len(proargnames) && proargnames[idx] != "" {
parameter.Name = proargnames[idx]
}

// Extract parameter type if available
if idx < len(proallargtypes) {
// Convert OID string to integer and look up type name
if oidStr := proallargtypes[idx]; oidStr != "" {
// Try to convert to integer OID first
if typeOid, err := strconv.ParseInt(oidStr, 10, 64); err == nil {
parameter.DataType = i.lookupTypeNameFromOID(typeOid)
} else {
// If it's not a number, treat as a direct type name
parameter.DataType = oidStr
}
}
}

parameters = append(parameters, parameter)
position++
}

return parameters
}

// lookupTypeNameFromOID converts PostgreSQL type OID to type name
func (i *Inspector) lookupTypeNameFromOID(oid int64) string {
Expand Down
5 changes: 1 addition & 4 deletions ir/queries/queries.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,10 +773,7 @@ SELECT
ELSE NULL
END AS volatility,
p.proisstrict AS is_strict,
p.prosecdef AS is_security_definer,
p.proargmodes::text[] as proargmodes,
p.proargnames,
p.proallargtypes::oid[]::text[] as proallargtypes
p.prosecdef AS is_security_definer
FROM information_schema.routines r
LEFT JOIN pg_proc p ON p.proname = r.routine_name
AND p.pronamespace = (SELECT oid FROM pg_namespace WHERE nspname = r.routine_schema)
Expand Down
10 changes: 10 additions & 0 deletions testdata/dump/issue_125_function_default/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "issue_125_function_default",
"description": "Test case for function parameter defaults being lost when dumping (GitHub issue #125)",
"source": "https://github.com/pgschema/pgschema/issues/125",
"notes": [
"Tests that function parameter default values are correctly preserved when dumping",
"Reproduces the bug where defaults like 'DEFAULT TRUE' and 'DEFAULT NULL' are lost",
"Tests both simple IN parameters with defaults and functions with OUT/INOUT parameters"
]
}
82 changes: 82 additions & 0 deletions testdata/dump/issue_125_function_default/pgdump.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
--
-- 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: test_simple_defaults(integer, text, boolean, numeric, timestamp without time zone); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION public.test_simple_defaults(param1 integer DEFAULT 42, param2 text DEFAULT 'hello world'::text, param3 boolean DEFAULT true, param4 numeric DEFAULT 3.14, param5 timestamp without time zone DEFAULT CURRENT_TIMESTAMP) RETURNS text
LANGUAGE plpgsql
AS $$
BEGIN
RETURN 'Params: ' || param1 || ', ' || param2 || ', ' || param3 || ', ' || param4 || ', ' || param5;
END;
$$;

--
-- Name: test_mixed_params(boolean, text, integer); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION public.test_mixed_params(raise_on_error boolean DEFAULT true, p_schema_override text DEFAULT NULL::text, max_retries integer DEFAULT 3, OUT success boolean, OUT message text) RETURNS record
LANGUAGE plpgsql
AS $$
BEGIN
success := NOT raise_on_error;
message := 'Schema: ' || COALESCE(p_schema_override, 'default') || ', Retries: ' || max_retries;
END;
$$;

--
-- Name: test_inout_params(integer, numeric, numeric); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION public.test_inout_params(value integer DEFAULT 100, INOUT multiplier numeric DEFAULT 1.5, INOUT result numeric DEFAULT NULL::numeric) RETURNS record
LANGUAGE plpgsql
AS $$
BEGIN
result := value * multiplier;
multiplier := multiplier * 2;
END;
$$;

--
-- Name: test_complex_defaults(integer[], jsonb, int4range, integer); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION public.test_complex_defaults(arr integer[] DEFAULT ARRAY[1, 2, 3], json_data jsonb DEFAULT '{"key": "value"}'::jsonb, range_val int4range DEFAULT '[1,10)'::int4range, expr_default integer DEFAULT 40) RETURNS jsonb
LANGUAGE plpgsql
AS $$
BEGIN
RETURN jsonb_build_object(
'arr', arr,
'json', json_data,
'range', range_val::text,
'expr', expr_default
);
END;
$$;

--
-- Name: test_variadic_defaults(text, integer[]); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION public.test_variadic_defaults(prefix text DEFAULT 'Result:'::text, VARIADIC numbers integer[] DEFAULT ARRAY[]::integer[]) RETURNS text
LANGUAGE plpgsql
AS $$
BEGIN
RETURN prefix || ' ' || array_to_string(numbers, ', ');
END;
$$;

--
-- PostgreSQL database dump complete
--
114 changes: 114 additions & 0 deletions testdata/dump/issue_125_function_default/pgschema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
--
-- pgschema database dump
--

-- Dumped from database version PostgreSQL 17.5
-- Dumped by pgschema version 1.4.0


--
-- Name: test_complex_defaults; Type: FUNCTION; Schema: -; Owner: -
--

CREATE OR REPLACE FUNCTION test_complex_defaults(
arr integer[] DEFAULT ARRAY[1, 2, 3],
json_data jsonb DEFAULT '{"key": "value"}',
range_val int4range DEFAULT '[1,10)',
expr_default integer DEFAULT 40
)
RETURNS jsonb
LANGUAGE plpgsql
SECURITY INVOKER
VOLATILE
AS $$
BEGIN
RETURN jsonb_build_object(
'arr', arr,
'json', json_data,
'range', range_val::text,
'expr', expr_default
);
END;
$$;

--
-- Name: test_inout_params; Type: FUNCTION; Schema: -; Owner: -
--

CREATE OR REPLACE FUNCTION test_inout_params(
value integer DEFAULT 100,
INOUT multiplier numeric DEFAULT 1.5,
INOUT result numeric DEFAULT NULL
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

The default value DEFAULT NULL is missing the explicit type cast. According to pgdump.sql line 49, this should be DEFAULT NULL::numeric to properly specify the typed null value.

Suggested change
INOUT result numeric DEFAULT NULL
INOUT result numeric DEFAULT NULL::numeric

Copilot uses AI. Check for mistakes.
)
RETURNS record
LANGUAGE plpgsql
SECURITY INVOKER
VOLATILE
AS $$
BEGIN
result := value * multiplier;
multiplier := multiplier * 2;
END;
$$;

--
-- Name: test_mixed_params; Type: FUNCTION; Schema: -; Owner: -
--

CREATE OR REPLACE FUNCTION test_mixed_params(
raise_on_error boolean DEFAULT true,
p_schema_override text DEFAULT NULL,
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

The default value DEFAULT NULL differs from the input in raw.sql which specifies DEFAULT NULL::text. The explicit cast to text should be preserved to match PostgreSQL's behavior and pg_dump's output (line 36 in pgdump.sql shows DEFAULT NULL::text).

Suggested change
p_schema_override text DEFAULT NULL,
p_schema_override text DEFAULT NULL::text,

Copilot uses AI. Check for mistakes.
max_retries integer DEFAULT 3,
OUT success boolean,
OUT message text
)
RETURNS record
LANGUAGE plpgsql
SECURITY INVOKER
VOLATILE
AS $$
BEGIN
success := NOT raise_on_error;
message := 'Schema: ' || COALESCE(p_schema_override, 'default') || ', Retries: ' || max_retries;
END;
$$;

--
-- Name: test_simple_defaults; Type: FUNCTION; Schema: -; Owner: -
--

CREATE OR REPLACE FUNCTION test_simple_defaults(
param1 integer DEFAULT 42,
param2 text DEFAULT 'hello world',
param3 boolean DEFAULT true,
param4 numeric DEFAULT 3.14,
param5 timestamp DEFAULT CURRENT_TIMESTAMP
)
RETURNS text
LANGUAGE plpgsql
SECURITY INVOKER
VOLATILE
AS $$
BEGIN
RETURN 'Params: ' || param1 || ', ' || param2 || ', ' || param3 || ', ' || param4 || ', ' || param5;
END;
$$;

--
-- Name: test_variadic_defaults; Type: FUNCTION; Schema: -; Owner: -
--

CREATE OR REPLACE FUNCTION test_variadic_defaults(
prefix text DEFAULT 'Result:',
VARIADIC numbers integer[] DEFAULT ARRAY[]::integer[]
)
RETURNS text
LANGUAGE plpgsql
SECURITY INVOKER
VOLATILE
AS $$
BEGIN
RETURN prefix || ' ' || array_to_string(numbers, ', ');
END;
$$;

Loading