Skip to content

fix: track LEAKPROOF and SECURITY DEFINER #181

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

Jump to bottom
Merged
tianzhou merged 14 commits into from
Nov 28, 2025
Merged

fix: track LEAKPROOF and SECURITY DEFINER #181

tianzhou merged 14 commits into from
Nov 28, 2025

Conversation

@tianzhou
Copy link
Contributor

@tianzhou tianzhou commented Nov 28, 2025

Fix #174

tianzhou and others added 13 commits November 28, 2025 08:44
@tianzhou @claude
docs: design for LEAKPROOF and PARALLEL function attributes
e8e4c46 
Add comprehensive design document for implementing PostgreSQL function
attributes LEAKPROOF and PARALLEL (SAFE/UNSAFE/RESTRICTED) support.

Covers:
- IR structure changes
- Database inspection from pg_catalog.pg_proc
- Dump output formatting (hybrid approach)
- Diff logic and ALTER FUNCTION generation
- Test case strategy

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
feat: add IsLeakproof and Parallel fields to Function IR
6db5508 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
feat: extract LEAKPROOF and PARALLEL from pg_catalog.pg_proc
ba99ef7 
- Add proleakproof and proparallel to GetFunctionsForSchema SQL query
- Generate updated Go code with IsLeakproof (bool) and ParallelMode fields
- Map proparallel values ('s', 'r', 'u') to Parallel field (SAFE, RESTRICTED, UNSAFE)
- Map proleakproof to IsLeakproof field in Function IR
- Verified code compiles successfully

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
feat: output LEAKPROOF and PARALLEL in function dumps
963ed0a 
- Add PARALLEL output logic (SAFE/RESTRICTED, skip UNSAFE default)
- Add LEAKPROOF output logic (output when true, skip NOT LEAKPROOF default)
- Reorder function attributes: LANGUAGE, Volatility, PARALLEL, LEAKPROOF, STRICT, SECURITY DEFINER

Task 3 of function LEAKPROOF and PARALLEL support implementation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
fix: align function attribute order with pg_dump canonical format
548d413 
Research of pg_dump source code (pg_dump.c lines 13686-13750) revealed
the canonical ordering for function attributes. This commit updates
pgschema to match pg_dump exactly.

Changes:
1. Move STRICT before SECURITY DEFINER (was after)
2. Move SECURITY DEFINER before LEAKPROOF (was after)
3. Remove SECURITY INVOKER output (it's the default, pg_dump never outputs it)
4. Keep PARALLEL SAFE/RESTRICTED at the end (after LEAKPROOF)

The correct pg_dump order is:
- LANGUAGE (always)
- VOLATILE/STABLE/IMMUTABLE (only non-default)
- STRICT (only if true)
- SECURITY DEFINER (only if true, INVOKER is default)
- LEAKPROOF (only if true)
- COST (only if non-default)
- ROWS (only if non-default)
- SUPPORT (only if set)
- PARALLEL SAFE/RESTRICTED (only non-default)

Updated internal/diff/function.go and all affected test fixtures.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
test: update test fixtures for function attribute ordering
a1c2ee7 
Update integration test fixtures (plan.txt, plan.json) after fixing
function attribute ordering to match PostgreSQL standards.

Changes:
- Regenerated function test fixtures using --generate flag
- Regenerated trigger test fixtures (depend on functions)
- Regenerated dependency test fixtures (function dependencies)
- Regenerated migrate test fixtures (include functions)
- Updated dump test expected outputs (employee, sakila, tenant, issue_125)
- Updated include test expected outputs (modular function files)

All tests now pass with correct attribute ordering that omits default
SECURITY INVOKER attribute and properly positions PARALLEL SAFE.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
test: add LEAKPROOF and PARALLEL to add_function test case
2bc407c 
Updated test case to demonstrate new LEAKPROOF and PARALLEL function
attributes:
- process_order: VOLATILE, PARALLEL RESTRICTED, LEAKPROOF, SECURITY DEFINER, STRICT
- days_since_special_date: STABLE, PARALLEL SAFE, LEAKPROOF
- safe_add: IMMUTABLE, PARALLEL SAFE, LEAKPROOF, STRICT (new function)

Updated all test fixture files (diff.sql, new.sql, plan.sql, plan.txt,
plan.json) to match the correct attribute ordering from the dump
formatter.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
feat: generate ALTER FUNCTION for LEAKPROOF and PARALLEL changes
299336b 
- Add IsLeakproof and Parallel to functionsEqual comparison
- Create functionsEqualExceptAttributes helper to detect attribute-only changes
- Generate ALTER FUNCTION PARALLEL {SAFE|UNSAFE|RESTRICTED} when parallel mode changes
- Generate ALTER FUNCTION [NOT] LEAKPROOF when leakproof attribute changes
- Use CREATE OR REPLACE only when function body or other attributes change

This enables efficient migrations that use ALTER FUNCTION for attribute changes
instead of recreating the entire function definition.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
test: add alter_function_attributes test case
a49c971 
This test validates that ALTER FUNCTION statements are correctly
generated when only LEAKPROOF and PARALLEL attributes change
(no function body changes).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
test: add alter_function_attributes test case and update fixtures
c8bfb75 
- Create new test case for ALTER FUNCTION attribute changes
- Regenerate add_function fixtures with LEAKPROOF and PARALLEL
- Validates ALTER FUNCTION generation for attribute-only changes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
docs: add implementation plan for LEAKPROOF and PARALLEL support
c694cd0 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou @claude
refactor: improve add_function test case clarity
81c7866 
Restructure test functions for better separation of concerns:
1. process_order - Complex case with all qualifiers (VOLATILE, STRICT, SECURITY DEFINER, LEAKPROOF, PARALLEL RESTRICTED)
2. calculate_tax - Tests PARALLEL SAFE only (IMMUTABLE, PARALLEL SAFE)
3. mask_sensitive_data - Tests LEAKPROOF only (STABLE, LEAKPROOF)

This makes it clearer what each function is testing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@tianzhou
chore: remove plan
1e3c1d2
Copilot AI review requested due to automatic review settings November 28, 2025 17:11
Copilot AI reviewed Nov 28, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements tracking of LEAKPROOF and SECURITY DEFINER function attributes to fix issue #174. The changes ensure that PostgreSQL function definitions properly capture and handle these important security and optimization attributes.

Key Changes

  • Added support for tracking LEAKPROOF and PARALLEL (SAFE/RESTRICTED/UNSAFE) function attributes from PostgreSQL system catalogs
  • Removed default output of SECURITY INVOKER (since it's the PostgreSQL default) and only output SECURITY DEFINER when explicitly set
  • Implemented smart ALTER FUNCTION statements for attribute-only changes (LEAKPROOF, PARALLEL) instead of always using CREATE OR REPLACE, preserving function permissions and dependencies

Reviewed changes

Copilot reviewed 64 out of 64 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
ir/queries/queries.sql Added proleakproof and proparallel columns to function metadata query
ir/queries/queries.sql.go Updated generated Go code with new IsLeakproof and ParallelMode fields
ir/ir.go Added IsLeakproof and Parallel fields to Function struct
ir/inspector.go Implemented logic to parse parallel mode from PostgreSQL's single-character representation and populate new function attributes
internal/diff/function.go Removed default SECURITY INVOKER output, added LEAKPROOF and PARALLEL output logic, and implemented ALTER FUNCTION for attribute-only changes
testdata/dump/*/pgschema.sql Updated schema dumps to reflect new function attribute handling and version bump to 1.4.3
testdata/diff/*/plan.json Updated migration plans with corrected function definitions and fingerprint hashes
testdata/diff/create_function/alter_function_attributes/* New test case demonstrating ALTER FUNCTION for LEAKPROOF and PARALLEL changes
testdata/diff/create_function/add_function/* Enhanced test case with comprehensive coverage of LEAKPROOF, PARALLEL SAFE, PARALLEL RESTRICTED, and SECURITY DEFINER
Various testdata diff files Updated expected outputs to remove SECURITY INVOKER (now default) from function definitions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

testdata/diff/migrate/v3/diff.sql Outdated
RETURNS trigger
LANGUAGE plpgsql
SECURITY INVOKER

Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

Unnecessary blank line between LANGUAGE plpgsql and VOLATILE. This blank line was left when SECURITY INVOKER was removed. The test file should be regenerated to remove this formatting issue.

Copilot uses AI. Check for mistakes.
@tianzhou tianzhou merged commit caa457e into main Nov 28, 2025
2 checks passed
@tianzhou tianzhou deleted the leakproof branch December 5, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Wrong order when using function for default value

1 participant

@tianzhou