PG19: reject FOR PORTION OF modifications planned by Citus
- Dominant language
- C
- Stars
- 12.8k
- Forks
- 794
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 31
Description
## Problem
PostgreSQL 19 adds temporal modifications:
```sql
UPDATE table_name FOR PORTION OF valid_at FROM ... TO ...;
DELETE FROM table_name FOR PORTION OF valid_at FROM ... TO ...;
```
When Citus plans/deparses these statements, it silently drops the `FOR PORTION OF` clause. PostgreSQL's rewritten overlap predicate and range intersection remain, but the executor-only leftover-row insertion phase is lost.
## Verified impact
Starting from one row valid over `[2020-01-01,2030-01-01)` and modifying/deleting `[2023-01-01,2025-01-01)`:
- Correct PostgreSQL UPDATE produces left, changed-middle, and right fragments.
- Citus remote UPDATE keeps only the middle fragment.
- Correct PostgreSQL DELETE preserves left and right fragments.
- Citus remote DELETE removes the entire overlapping row.
- RETURNING and command tags remain plausible, making the loss difficult to detect.
- Remote execution also omits the INSERT trigger cycles PostgreSQL fires for leftover rows.
Reproduced across router, multi-shard, single-shard, reference, Citus-local, partitioned, modifying-CTE, and ordinary-local-target/distributed-source paths. Local execution can appear correct accidentally; forcing remote execution exposes deterministic loss.
If the temporal range is the distribution column, leftover fragments can belong to different shards, so simply restoring clause deparsing would still be unsafe.
## Root cause
PG19 `Query.forPortionOf` is carried to the PostgreSQL executor, which locks the original tuple and inserts untouched left/right ranges. Citus's PG19 ruleutils copy does not emit the temporal clause and sends an ordinary rewritten UPDATE/DELETE to workers. The worker executor therefore never runs temporal leftover handling.
## Severity
High PG19 support release blocker: valid SQL causes deterministic silent data loss/corruption without an error.
## Proposed minimum fix
Explicitly reject every PG19 temporal modification that enters Citus modification planning:
- Add a PG19-gated check for `queryTree->forPortionOf != NULL` in `ModifyPartialQuerySupported()` before routing/target-list validation.
- Return `ERRCODE_FEATURE_NOT_SUPPORTED` with a stable message.
- Do not create router-only, reference, Citus-local, local-execution, or partitioned exceptions.
- Pure PostgreSQL local-table statements that never enter Citus planning remain supported.
## Acceptance criteria
- Ordinary PostgreSQL local temporal UPDATE/DELETE remains correct.
- Distributed temporal UPDATE and DELETE reject consistently with local execution on/off.
- Modifying CTEs reject consistently.
- Ordinary local targets that read distributed sources reject consistently.
- Temporal DELETE with the period as distribution column rejects before any row loss.
- PG17/18 builds and behavior remain unchanged through version-gated field access.
- PG19 regression coverage verifies the rejection boundaries and unchanged local behavior.
## Non-goals
Real distributed support requires preserving the original temporal clause, shard-correct leftover routing, reference replication, partition tuple routing, EvalPlanQual/locking, trigger semantics, and RETURNING behavior. It is out of scope for this rejection fix.
## Scope clarifications
- PostgreSQL 19 grammar permits `FOR PORTION OF` on UPDATE and DELETE only. MERGE and its reduced action syntax do not accept the clause, so no `CreateMergePlan()` guard is needed.
- The one PG19-gated check in `ModifyPartialQuerySupported()` covers both supported temporal target spellings:
- `FROM start_time TO end_time`
- `(portion)`
- PostgreSQL inserts zero to two range leftovers, or at most one multirange leftover. Leftovers preserve original values in non-period columns, including the distribution column; the verified Citus defect is silent **loss** of leftover rows after deparsing, not demonstrated cross-shard misrouting.
- PostgreSQL does not require INSERT privilege for these executor-generated leftovers. Any future real support must preserve that privilege behavior; the rejection fix does not emulate leftovers.
## Accepted local implementation
Local commit `9453db38c` implements the approved minimum rejection in `ModifyPartialQuerySupported()` with mandatory PG19 gating and focused PG19 regression coverage for both temporal target forms and all validated Citus planning boundaries.
Validation:
- PG17/18/19 `-Werror` builds succeeded.
- PG19 and PG18 focused base/planner runs passed 17/17.
- PG17 relevant `pg19`, `multi_modifications`, and `citus_local_tables_queries` tests passed.
- `make check-style`, final diff checks, and independent release review passed.
No MERGE guard is needed because PostgreSQL 19 grammar does not admit `FOR PORTION OF` in MERGE.
Implementation PR: #8763.
Contributor guide
Research direction
Read ModifyPartialQuerySupported() and the PG19 regression coverage referenced in the issue, including pg19, multi_modifications, and citus_local_tables_queries. Compare the accepted local commit and implementation PR #8763; done means consistent rejection at the Citus planning boundaries while ordinary local temporal UPDATE/DELETE and PG17/18 behavior remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, postgresql, sql
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 15/100