Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements procedure mode support to enhance PostgreSQL procedure parameter handling. The changes ensure that procedure parameters include explicit mode specifications (IN, OUT, INOUT) in both creation and drop statements, matching pg_dump behavior for better PostgreSQL compatibility.
- Adds explicit parameter mode handling in procedure parsing and generation
- Updates DROP PROCEDURE statements to include full parameter signatures with modes and names
- Adds structured parameter support with new Signature and Parameters fields
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/dump/employee/pgschema.sql | Updates procedure definition to include explicit IN mode for parameters |
| testdata/diff/migrate/v5/* | Updates DROP PROCEDURE statements to use full parameter signatures instead of type-only |
| testdata/diff/create_procedure/* | Updates test cases to use explicit parameter modes and full signatures |
| ir/parser.go | Adds signature generation with explicit parameter modes and structured parameter support |
| ir/inspector.go | Enhances parameter parsing to handle mode keywords (IN, OUT, INOUT, etc.) |
| internal/diff/procedure.go | Replaces type-only parameter extraction with full signature formatting for DROP statements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if param.Mode != "" && param.Mode != "IN" { | ||
| // Only include non-default modes explicitly (OUT, INOUT, VARIADIC) | ||
| sigPart = param.Mode + " " | ||
| } else { | ||
| // Always include IN mode explicitly for clarity | ||
| sigPart = "IN " | ||
| } |
There was a problem hiding this comment.
The comment on line 1877 contradicts the implementation. The comment states 'Only include non-default modes explicitly' but the code always includes 'IN' mode explicitly in the else branch, which contradicts the 'only' statement.
e3bd6b9 to
f5a039b
Compare
f5a039b to
9321020
Compare
Fix #65