feat: warn about and skip statements that have no effect on the plan (#602, #603)

[tianzhou]

Summary

Two statement kinds set state that pgschema's inspection never reads, so a schema file containing them produced a successful empty plan with no diagnostic:

• ALTER <object> ... OWNER TO — ownership follows the pg_dump --no-owner model (Support object OWNER changes or diagnose ignored ownership intent #602)
• ALTER DEFAULT PRIVILEGES without IN SCHEMA — a database-wide default ACL, not schema-level state (Diagnose unsupported global ALTER DEFAULT PRIVILEGES instead of returning an empty plan #603)

plan (and apply, which goes through GeneratePlan) now prints a warning per statement on stderr and drops it from the desired-state SQL:

Warning: statement has no effect: ALTER TABLE item OWNER TO app_owner
  pgschema does not manage object ownership; change the owner outside pgschema, see https://www.pgschema.com/syntax/unsupported#object-ownership

Why drop, not just warn

• A global ALTER DEFAULT PRIVILEGES runs for real on the plan database. On an external plan database nothing cleans it up, so it would persist past the run.
• A role named only in OWNER TO is not stubbed, so the statement fails in the plan database with role "x" does not exist.

Stripping happens right after include processing, before role validation and before the plan database.

How

• One detector, postgres.StripNoEffectStatements, returns the findings plus the SQL without them. Adding another kind is one more case in classifyNoEffect.
• Best-effort textual scan, no parser. It reuses the existing scanner (walkSQLCode*, parseQuotedIdent, …); walkSQLCodeSpans is a small refactor that exposes byte offsets. String literals, comments and dollar-quoted bodies are skipped, so statements inside DO blocks or function bodies are not seen.
• Removal blanks the statement in place (length and newlines preserved), so PostgreSQL error positions still map to the user's file.
• OWNER TO combined with other actions in one ALTER TABLE is warned about but kept.
• RENAME [COLUMN|CONSTRAINT|ATTRIBUTE] owner TO ... and objects named owner are not misread. ALTER SCHEMA/DATABASE ... OWNER TO is left alone.

Docs

• Adds the "Global Default Privileges" section to the Unsupported page and a warning on the ALTER DEFAULT PRIVILEGES page (Diagnose unsupported global ALTER DEFAULT PRIVILEGES instead of returning an empty plan #603 counterpart of docs: document that object ownership is not managed (#602) #620).
• Updates the "Object Ownership" section to describe the warning.

Test plan

• go test ./internal/postgres/ — new TestStripNoEffectStatements table test
• go test ./cmd/plan/ — new TestPlan_WarnsAboutNoEffectStatements (embedded Postgres; confirmed it fails without the fix)
• go test ./internal/diff/
• PGSCHEMA_TEST_FILTER="default_privilege/" go test ./cmd -run TestPlanAndApply
• Full go test ./... left to CI

Closes #602
Closes #603

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 3/5

The PR is not safe to merge until combined ownership actions are prevented from executing in the plan database and index ownership statements receive the intended warning and stripping behavior.

Findings

1.  Combined ownership still executes ▶
2.  Index ownership bypasses detection ▶

Summary

This PR adds preprocessing that warns about and blanks desired-state statements whose ownership or global-default-privilege state is not inspected, before role validation and plan-database execution.

• Adds textual detection and position-preserving removal of standalone no-effect statements.
• Integrates warnings into file-based plan generation and apply.
• Documents unsupported ownership and database-wide default privileges.
• The ownership handling remains incomplete for indexes and does not safely handle comma-separated ownership actions.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Expanded desired SQL] --> B[Detect no-effect statements]
  B -->|Standalone owner or global defaults| C[Warn and blank statement]
  B -->|Other SQL| D[Retain statement]
  C --> E[Validate referenced roles]
  D --> E
  E --> F[Apply to plan database]
  F --> G[Inspect desired state]
  G --> H[Generate plan]

Loading

_{Reviews (1) · Last reviewed commit: "feat: warn about and skip statements tha..."}

[Copilot]

Copilot review overview

🟡 Changes recommended

The scanner can alter valid E-string contents and misclassify or miss valid ownership statements.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 2 · 1

Open (4)

• E-string escaped quotes cause destructive masking of valid SQL · New
• Foreign data wrapper ownership statements are incorrectly removed · New
• Ownership detection fails for objects named introducer keywords · New
• Warning incorrectly claims retained multi-action statements have no effect · New

What changed in this PR

Adds warnings and filtering for unsupported ownership changes and global default privileges.

Changes:

• Detects and removes no-effect SQL while preserving source offsets.
• Integrates warnings into plan/apply generation.
• Adds tests and unsupported-syntax documentation.

File	Description
internal/​postgres/​no_effect.go	Implements detection and stripping.
internal/​postgres/​no_effect_test.go	Adds detector tests.
internal/​postgres/​fk_refs.go	Exposes code-span offsets.
cmd/​plan/​plan.go	Applies stripping during planning.
cmd/​plan/​no_effect.go	Formats warnings.
cmd/​plan/​no_effect_integration_test.go	Tests planning behavior.
docs/​syntax/​unsupported.mdx	Documents unsupported statements.
docs/​syntax/​alter_default_privileges.mdx	Documents IN SCHEMA requirement.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

The scanner can corrupt valid ordinary strings when standard_conforming_strings is disabled.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: None

Resolved since last review (4)

• E-string escaped quotes cause destructive masking of valid SQL
• Ownership detection fails for objects named introducer keywords
• Foreign data wrapper ownership statements are incorrectly removed
• Warning incorrectly claims retained multi-action statements have no effect