Hi Hackers,
Noah Misch found a number of new FOR PORTION OF bugs with some LLM
investigation. I've attached the original prompt and findings. Here is
a list with my own description of each:
D1: A DO INSTEAD rule skips inserting leftovers. (Already reported by [0].)
D2: PREPARE requires casting parameters in FOR PORTION OF bounds (e.g.
$1::date), but we should coerce these automatically.
D3: EXPLAIN (GENERIC_PLAN) fails with `no value found for parameter 1`
D4: EXPLAIN with `FOR PORTION OF (null)` raises an error about the null target.
D5: elog if the range column's default opclass is for a concrete range
type, not anyrange.
D6: Under plain inheritance, statement-level insert triggers for
temporal leftovers fire against the parent table, not the child.
D7: Error hint is misleading if a temporal leftover's BEFORE trigger
modifies a row that is later changed by the top-level statement.
D8: A temporal leftover is not inserted if its BEFORE INSERT trigger
returns null. (Already reported by [1].)
D9: FOR PORTION OF bound expressions are evaluated more than once. A
function declared STABLE but not actually STABLE would get
inconsistent results. Also reachable with current_setting() +
set_config().
D10: EXPLAIN evaluates functions in the FOR PORTION OF bounds.
D11: A WITH CHECK OPTION failure blames the temporal leftover when it
should blame the original update.
D12: pg_get_ruledef deparses a null bound as NULL::unknown not NULL::date.
D13: Docs should clarify that you need permission on the range column.
D14: Error message is misleading for a view referencing the range column twice.
I've got some patches to fix all these (with D2+D12 grouped and
D3+D4+D10 grouped), but I'll make a separate thread for each fix. I
just wanted to have this message as something I could reference from
there.
[0]
https://www.postgresql.org/message-id/flat/CAON2xHO5FJ1jC87cQhGg%2BKMG5QR7ZaifJwN24rPjMqCGYn-hzA%40mail.gmail.com
[1]
https://www.postgresql.org/message-id/flat/CAN4CZFNSOA_LXWKC6sAOduWVsaxjzmFBNEHUQii89KUnMhBtcw%40mail.gmail.com
Yours,
--
Paul ~{:-)
[email protected]
# PROVENANCE
This branch was produced by an AI agent. Everything below records how.
## Model and tooling
| | |
|---|---|
| Orchestrator model | Claude Opus 5 (`claude-opus-5`) |
| Hunt sub-agent model | Claude Sonnet 5 (`claude-sonnet-5`) |
| Harness | GitHub Copilot CLI, version 1.0.82 |
| Date of run | 2026-09-03 (UTC) |
| Host | Linux, 2 CPUs, 15 GB RAM |
## Prompt
The task was given by user `@nmisch` in a single message, reproduced verbatim:
> Make a large workflow, with at most 270 agents, to write test cases covering
> user-visible defects in the new FOR PORTION OF feature (commit 8e72d91) that
> are still present in master. Use your own worktree; disregard the present dir
> except as repository to which to attach your worktree. The workflow should
> first look for extant user-visible defects. If it finds any, write a test
> case covering some of those defects. If any defects found weren't suitable to
> test, describe them in a report.
>
> Commit the following on a fresh branch:
> - A report describing any defects found, testable or not. Prefix the report
> with [no defects] if that's so.
> - Any tests written
> - A PROVENANCE.md file containing model, prompt, etc.
No other human input, guidance, or correction was given at any point.
## Subject under test
| | |
|---|---|
| Feature commit | `8e72d914c52876525a90b28444453de8085c866f` — "Add UPDATE/DELETE FOR PORTION OF" (2026-04-01) |
| Baseline | `master` = `0b776de09ed3e237181def135577321bdd31b548` (2026-09-03) |
| Version string | PostgreSQL 20devel |
| Branch created | `copilot/8e72d91-for-portion-of-defects`, branched from `0b776de` |
## Workflow
**1. Isolated worktree and build.** A dedicated worktree was attached to the
repository found in the working directory; nothing in that directory was
modified. The worktree was configured with meson
(`-Dcassert=true -Ddebug=true -Doptimization=0 -Dtap_tests=enabled`) so that
any `Assert` reachable from SQL would trip, and installed to a private prefix.
**2. Sandbox harness.** A per-agent PostgreSQL sandbox was built: a
pre-`initdb`'d template cluster plus a `pgenv.sh` driver giving each agent an
isolated slot (`start`/`stop`/`reset`/`sql`/`psql`/`log`). Each slot uses its
own data directory as its Unix socket directory, so agents could run clusters
concurrently with no port or path collisions.
**3. Ground-truth survey.** All follow-up commits to `8e72d91` were
enumerated and checked with `git merge-base --is-ancestor`. All 19 are
already contained in `0b776de`, so no already-fixed bug could be
mistaken for a live one. The list was written into the agent briefing as a
"do not report these" section, together with a definition of "user-visible
defect" (something a user can observe through SQL, client protocol,
documentation, or error output — not an internal code smell), a requirement
that every finding carry a control case proving the non-temporal equivalent
behaves correctly, and a fixed output format for findings.
**4. Decomposition.** The feature's surface was decomposed into **82 hunt
areas** (parser, rewriter, planner, executor, triggers, partitioning,
inheritance, views and rules, RLS and privileges, constraints, temporal keys,
concurrency, deparse, documentation claims, error-message audit, and an
"incomplete fix" audit of each of the 19 follow-up commits). Each area became
a self-contained assignment file.
**5. Agent waves.** Hunt agents were launched in waves against those
assignments — **33 agents in total**, well inside the 270 budget. The runtime
enforces a hard cap of 32 concurrent background agents, which was reached; the
remaining 49 hunt areas were therefore never dispatched.
**6. Observed limitation, and the pivot.** At 32-way concurrency the fleet
was throughput-starved: for roughly the first 70 minutes, 32 of the 33 agents
completed zero turns and issued zero tool calls, while the one agent launched
when only eight were running finished normally in 31 minutes. Rather than
wait, the orchestrator hunted directly, in parallel with the stalled fleet,
and committed an initial report and test set covering the three defects it had
found on its own.
The fleet then unblocked — once the orchestrator stopped competing for
throughput, agents began finishing at a steady rate, each taking roughly
4,400–6,000 seconds end to end. **32 of the 33 agents returned a final
report** before this branch was finalized; the 33rd (NULL/empty ranges) had
already written its findings file, recording `NONE`, but had not yet returned
its summary. All results were triaged afterwards and substantially expanded
the report — including the most serious defect in it.
The practical lesson, recorded here because it shaped the result: for this
runtime, agent fan-out should be sized at roughly 8–12 concurrent agents. A
32-wide fleet does eventually deliver, but it front-loads a long dead period
in which no agent makes progress.
**7. Findings accounting.** Of the 33 agents, **23 reported `NONE`** and 10
reported at least one candidate defect. Attribution of the 14
findings in the report:
| Source | Findings |
|---|---|
| Hunt agents | D1 (rules on views — the most serious), D5, D6, D7, D8, D9, D11 |
| Orchestrator | D2, D3, D4, D10, D12, D13, D14 |
| Both, independently | D2 (agent h80 rediscovered it with the same root cause); D5 (agents h57 and h64 found it independently of each other) |
**8. Triage and false positives.** Agent findings were **not** taken on
trust. Every claim was re-derived by hand on an unmodified `0b776de` before
being written into the report, and the ones that did not survive are listed in
the report's "Rejected candidate findings" section. This mattered: one
finding (h27-1) was a false positive — the behaviour it described is already
covered by an existing test that deliberately captures it — and one (h57-1)
could not be reproduced from the agent's summary at all until the agent was
asked, in a follow-up turn, for its exact script. The missing ingredient was
that GiST support function 12 must be registered with `("any","any")`
lefttype/righttype rather than the concrete range type; with that detail the
`XX000` reproduced immediately. Roughly one in five agent findings needed
either correction or outright rejection.
**9. Method.** Differential testing against independently computed ground
truth, plus systematic comparison of each `FOR PORTION OF` construct against
its hand-written non-temporal equivalent. Notably, ~700 combinations of row
range × target bounds were compared against `range_intersect` /
`multirange_minus` results (no mismatch — the core range algebra is sound),
which redirected the search toward the feature's boundaries with other
subsystems. That is where every reported defect lives: the rewriter's
interaction with rules, `EXPLAIN`, parameter type resolution, trigger timing,
inheritance, and opclass assumptions.
**10. Verification.** Each finding was reproduced from a clean cluster with
an explicit control case demonstrating that the non-temporal equivalent
behaves correctly. Candidate root-cause fixes were then applied
**temporarily** to confirm the diagnosis and to generate trustworthy expected
output for the new tests — the expected output was *generated from a working
fix*, never hand-written — verified to leave all 2795 lines of pre-existing
expected output unchanged, and then reverted. The committed tree contains
**no source changes**: `git diff` against `0b776de` touches only
`src/test/regress/`, the report, and this file.
## What is committed
| File | Contents |
|---|---|
| `FOR-PORTION-OF-DEFECTS.md` | The report: 4 tested defects, 10 described-but-untested findings, root-cause analysis, rejected candidates, and a coverage list of areas probed without finding anything |
| `src/test/regress/sql/for_portion_of.sql` | Two new test blocks: `fpo_param` (parameter type inference; `EXPLAIN` and `EXPLAIN (GENERIC_PLAN)`) and `fpo_instead_rule` (`FOR PORTION OF` on views with `INSTEAD`/`INSTEAD NOTHING`/`ALSO` rules) |
| `src/test/regress/expected/for_portion_of.out` | Matching expected output, encoding **correct** behaviour |
| `PROVENANCE.md` | This file |
## How to read the test result
The new tests deliberately encode correct behaviour, so **they fail on
`0b776de`**, and that failure diff *is* the defect demonstration:
```
build/src/test/regress/pg_regress --bindir=<prefix>/bin \
--inputdir=src/test/regress --expecteddir=src/test/regress \
--dlpath=build/src/test/regress --temp-instance=/tmp/ti for_portion_of
```
produces a 190-line, 5-hunk diff: 11 unexpected `ERROR` lines (D2/D3/D4), 3
missing `ERROR` lines (D1), and the wrong row contents that D1 leaves behind —
a renamed whole row and, for `DELETE`, an empty table. All hunks are confined
to the two new test blocks. With the three candidate fixes described in the
report's "Root causes" section applied, the test passes.
## Caveats
* 49 of the 82 identified hunt areas were never dispatched, because the
runtime's 32-agent concurrency cap was reached. They should be considered
**unexamined**, not clean.
* The report's "areas examined without finding a defect" list is assembled
from the orchestrator's own probes and from 23 agents that reported `NONE`.
Those agents' negative results were spot-checked for plausibility but, being
negatives, were not independently re-derived the way the positive findings
were.
* Ten of the fourteen findings are described rather than tested. In most
cases that is a deliberate judgement recorded in the report: the correct
behaviour is a design question whose resolution would change the expected
output, so pinning today's behaviour would enshrine a bug and pinning a
guess would prejudge the fix.
* Concurrency and crash-recovery behaviour was only lightly probed; the
existing isolation spec documents a known `READ COMMITTED` limitation that
was treated as documented behaviour rather than a defect.
# User-visible defects in `FOR PORTION OF` still present in master
Feature under test: commit `8e72d91` ("Add UPDATE/DELETE FOR PORTION OF",
2026-04-01).
Baseline: `master` = `0b776de` (2026-09-03), PostgreSQL 20devel, built with
`-Dcassert=true -Ddebug=true -Doptimization=0`.
Four user-visible defects were found and are covered by new regression tests
in `src/test/regress/{sql,expected}/for_portion_of.sql`. Nine further
findings that were not suitable for a regression test are described in
"Defects not covered by a test" below.
All 19 published follow-up fixes to `8e72d91` are already contained in
`0b776de` (verified with `git merge-base --is-ancestor`), so none of the
findings below is an already-fixed bug.
Every finding below was reproduced by hand on an unmodified `0b776de` before
being written down. Findings reported by the hunt agents that did *not*
survive that check are listed in "Rejected candidate findings".
---
## Summary
| # | Defect | Severity | Tested |
|---|--------|----------|--------|
| D1 | `FOR PORTION OF` is **silently ignored** on a view with an `INSTEAD` rule: the whole row is overwritten, or the whole row is deleted | **Data loss** | yes |
| D2 | `FROM`/`TO` bounds do not resolve untyped parameters, breaking `PREPARE` and the extended query protocol | High | yes |
| D3 | `EXPLAIN (GENERIC_PLAN)` fails with `no value found for parameter 1` | Medium | yes |
| D4 | Plain `EXPLAIN` raises the run-time error `FOR PORTION OF target must not be null` | Medium | yes |
| D5 | `elog(ERROR, "unexpected opcintype")` — an `XX000` internal error — for a range column whose default GiST opclass has a concrete `opcintype` | High | no |
| D6 | Statement-level `INSERT` triggers for leftovers fire on the wrong table under plain inheritance | Medium | no |
| D7 | `AFTER` row triggers on leftovers fire mid-statement, causing a spurious error whose `HINT` cannot be followed | Medium | no |
| D8 | A `BEFORE INSERT` trigger returning `NULL` silently discards leftovers, destroying data the statement was supposed to preserve | Medium | no |
| D9 | Bound expressions are evaluated four times per row at three independent sites; divergent values yield overlapping or gapped history | Medium | no |
| D10 | Plain `EXPLAIN` evaluates the target expression, running user functions | Low | no |
| D11 | `WITH CHECK OPTION` violations blame the leftover row instead of the row the user's `SET` produced | Low | no |
| D12 | `pg_get_ruledef` deparses a `NULL` bound as `NULL::unknown` | Cosmetic | no |
| D13 | `UPDATE`/`DELETE` reference pages omit the privileges `FOR PORTION OF` requires on the range column | Doc | no |
| D14 | Misleading `multiple assignments to same column` error through a view that exposes the range column twice | Low | no |
---
## Defects covered by a test
### D1 — `FOR PORTION OF` is silently ignored on a view with an `INSTEAD` rule
This is the most serious finding: it destroys data with no error and a
command tag that claims success.
```sql
CREATE TABLE t (id int, valid_at daterange, name text);
INSERT INTO t VALUES (1, daterange('2018-01-01', '2020-01-01'), 'one');
CREATE VIEW v AS SELECT * FROM t;
CREATE RULE v_upd AS ON UPDATE TO v
DO INSTEAD UPDATE t SET name = NEW.name WHERE id = OLD.id;
CREATE RULE v_del AS ON DELETE TO v
DO INSTEAD DELETE FROM t WHERE id = OLD.id;
UPDATE v FOR PORTION OF valid_at FROM '2018-06-01' TO '2019-01-01' SET name = 'foo';
UPDATE 1
SELECT * FROM t;
id | valid_at | name
----+-------------------------+------
1 | [2018-01-01,2020-01-01) | foo -- the whole row was renamed
```
The row was not split and its range was not truncated: `FOR PORTION OF` had
no effect whatsoever. `DELETE` is worse — it removes the entire row instead
of carving out the requested portion:
```sql
DELETE FROM v FOR PORTION OF valid_at FROM '2018-06-01' TO '2019-01-01';
DELETE 1
SELECT * FROM t;
id | valid_at | name
----+----------+------
(0 rows) -- the row is gone entirely
```
`DO INSTEAD NOTHING` likewise swallows the clause. The identical statement
against a plain auto-updatable view (no rule) behaves correctly and produces
the expected three-way split, which is what makes this a silent trap rather
than an obvious limitation.
Impact: a temporal history table exposed through a rule-based updatable view
loses history. There is no error, no warning, and the command tag reports
one affected row, so nothing signals to the application that the temporal
qualifier was dropped.
The test asserts the clause is *rejected*, mirroring the error PostgreSQL
already raises for the analogous unsupported case (`views with INSTEAD OF
triggers do not support FOR PORTION OF`, commit `dfce19c`). A `DO ALSO`
rule leaves the original query in place and is verified to keep working.
### D2 — `FOR PORTION OF ... FROM $1 TO $2` cannot infer parameter types
`PREPARE` (and, equivalently, `PQprepare`/`PQexecParams` with unspecified
parameter OIDs, which is what most client drivers send) fails:
```sql
PREPARE p AS UPDATE t FOR PORTION OF valid_at FROM $1 TO $2 SET name = 'q';
ERROR: could not determine data type of parameter $1
LINE 1: PREPARE p AS UPDATE t FOR PORTION OF valid_at FROM $1 TO $2 ...
^
```
Every comparable construct resolves the parameter type from context:
```sql
-- the hand-written equivalent of the same clause: works
PREPARE c AS UPDATE t SET name = 'q' WHERE valid_at && daterange($1, $2);
SELECT parameter_types FROM pg_prepared_statements WHERE name = 'c';
{date,date}
-- the other FOR PORTION OF syntax: also works
PREPARE q AS UPDATE t FOR PORTION OF valid_at ($1) SET name = 'q';
{daterange}
```
`DELETE` is affected identically, and a single parameterized bound
(`FROM $1 TO '2003-01-01'`) is enough to trigger it.
Impact: the documented `FROM`/`TO` spelling of `FOR PORTION OF` — the one
shown in the `UPDATE` and `DELETE` reference pages and in `dml.sgml` — cannot
be used with parameters unless the client declares the parameter types
explicitly. The workaround is `PREPARE p(date, date) AS ...` or an explicit
cast on each bound.
### D3 — `EXPLAIN (GENERIC_PLAN)` fails
```sql
EXPLAIN (COSTS OFF, GENERIC_PLAN)
UPDATE t FOR PORTION OF valid_at ($1::daterange) SET name = 'q';
ERROR: no value found for parameter 1
```
`GENERIC_PLAN` exists specifically so that a statement containing parameter
placeholders can be planned with no parameter values supplied, and it works
for the identical qual written by hand:
```sql
EXPLAIN (COSTS OFF, GENERIC_PLAN)
UPDATE t SET name = 'q' WHERE valid_at && $1::daterange;
Update on t
-> Seq Scan on t
Filter: (valid_at && $1)
```
`DELETE ... FOR PORTION OF` is affected identically. (With the `FROM`/`TO`
spelling the statement fails earlier, with D2's error.)
### D4 — Plain `EXPLAIN` raises a run-time error
```sql
EXPLAIN (COSTS OFF)
UPDATE t FOR PORTION OF valid_at (NULL::daterange) SET name = 'q';
ERROR: FOR PORTION OF target must not be null
```
`EXPLAIN` without `ANALYZE` must not execute the statement, so it should
print the plan; the null check belongs to execution only. `DELETE` behaves
the same way. Executing the statement (correctly) still raises the error.
---
## Defects not covered by a test
### D5 — `XX000 unexpected opcintype` from ordinary SQL
`transformForPortionOfClause()` switches on the range column's default GiST
opclass `opcintype` and assumes it is polymorphic (`anyrange` /
`anymultirange`), falling through to `elog(ERROR, "unexpected opcintype")`
otherwise. A user-defined default opclass over a *concrete* range type is
perfectly legal and reaches that branch:
```sql
CREATE FUNCTION my_int4range_overlaps(int4range, int4range) RETURNS bool AS
$$ SELECT $1 OPERATOR(pg_catalog.&&) $2 $$ LANGUAGE SQL IMMUTABLE STRICT;
CREATE OPERATOR && (leftarg = int4range, rightarg = int4range,
procedure = my_int4range_overlaps, commutator = &&,
restrict = rangesel, join = areajoinsel);
CREATE OPERATOR CLASS my_int4range_gist_ops
DEFAULT FOR TYPE int4range USING gist AS
OPERATOR 3 &&(int4range, int4range),
FUNCTION 1 range_gist_consistent(internal, anyrange, smallint, oid, internal),
FUNCTION 2 range_gist_union(internal, internal),
FUNCTION 5 range_gist_penalty(internal, internal, internal),
FUNCTION 6 range_gist_picksplit(internal, internal),
FUNCTION 7 range_gist_same(anyrange, anyrange, internal);
-- must be registered as ("any","any"): gistutil.c looks it up with
-- get_opfamily_proc(opfamily, ANYOID, ANYOID, GIST_TRANSLATE_CMPTYPE_PROC)
ALTER OPERATOR FAMILY my_int4range_gist_ops USING gist
ADD FUNCTION 12 ("any", "any") gist_translate_cmptype_common(integer);
CREATE TABLE t57 (id int, valid_at int4range, name text);
INSERT INTO t57 VALUES (1, int4range(1,10), 'a');
UPDATE t57 SET name = 'c' WHERE valid_at && int4range(3,5); -- UPDATE 1, fine
UPDATE t57 FOR PORTION OF valid_at FROM 3 TO 5 SET name = 'b';
ERROR: XX000: unexpected opcintype: 3904
LOCATION: transformForPortionOfClause, analyze.c:1536
```
`DELETE ... FOR PORTION OF` fails identically. `XX000` with an
`elog`-style message is by convention a "can't happen" internal error, so
this is a user-reachable internal error. The control case above proves the
custom opclass itself works.
**Why it is not tested:** the correct behaviour is a genuine design question,
not an obvious output. PostgreSQL could support concrete-typed range
opclasses here, or reject them with a proper `ereport`; those two resolutions
have completely different expected output, and pinning either one would
prejudge the fix. Pinning the *current* `XX000` text would enshrine a bug.
The repro above is self-contained and reproduces in seconds.
Independently found by two hunt agents (h57, h64) and then reproduced by
hand.
### D6 — Statement triggers for leftovers fire on the wrong table (inheritance)
Since `7d13b03`, leftovers are correctly re-inserted into the *child* table
they came from, but `fireBSTriggers()`/`fireASTriggers()` still use
`rootResultRelInfo`, so statement-level `INSERT` triggers fire on the parent:
```sql
CREATE TABLE iparent (id int, valid_at daterange, name text);
CREATE TABLE ichild () INHERITS (iparent);
INSERT INTO ichild VALUES (1, daterange('2000-01-01','2010-01-01'), 'one');
CREATE TRIGGER iparent_ai AFTER INSERT ON iparent FOR EACH STATEMENT
EXECUTE FUNCTION stmt_trg();
CREATE TRIGGER ichild_ai AFTER INSERT ON ichild FOR EACH STATEMENT
EXECUTE FUNCTION stmt_trg();
INSERT INTO ichild VALUES (2, daterange('2000-01-01','2001-01-01'), 'ctl');
NOTICE: statement AFTER INSERT fired on ichild -- control: correct
UPDATE iparent FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01'
SET name = 'x';
NOTICE: statement AFTER INSERT fired on iparent
NOTICE: statement AFTER INSERT fired on iparent -- wrong table, twice
```
The rows demonstrably land in `ichild` (`SELECT tableoid::regclass ...`
confirms all three resulting rows are `ichild` rows), so a per-table audit
trigger on `ichild` silently misses the leftover inserts while a trigger on
`iparent`, which receives no rows at all, fires spuriously.
**Why it is not tested:** correct behaviour is under-determined. It is not
obvious whether one statement-level firing per leftover is intended at all
(the current design fires them once per leftover, which is itself arguably
wrong — see D7), so a test would have to pin both the *table* and the
*number* of firings, and only the table is clearly wrong. Fixing the target
relation without settling the firing count would leave the test asserting
half-specified behaviour.
### D7 — `AFTER` row triggers on leftovers fire mid-statement
`ExecForPortionOfLeftovers()` wraps every leftover insert in its own
`AfterTriggerBeginQuery()`/`AfterTriggerEndQuery()` pair
(`nodeModifyTable.c:1616-1621`) so that each leftover gets its own transition
table. A side effect is that `AFTER ... FOR EACH ROW` triggers on those
inserts run *while the outer statement is still scanning*, which breaks the
guarantee that `AFTER` row triggers observe the statement's final state.
The visible consequence is a hard error that the user cannot act on:
```sql
CREATE FUNCTION sib() RETURNS TRIGGER LANGUAGE plpgsql AS $$
BEGIN
IF pg_trigger_depth() > 1 THEN RETURN NULL; END IF;
EXECUTE format('UPDATE %I SET touched = touched + 1 WHERE id <> $1',
TG_TABLE_NAME) USING NEW.id;
RETURN NULL;
END $$;
CREATE TRIGGER ai_ai AFTER INSERT ON ai FOR EACH ROW EXECUTE FUNCTION sib();
UPDATE ai FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01'
SET name = name || '!';
ERROR: tuple to be updated was already modified by an operation triggered by the current command
HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.
```
The `HINT` is impossible to follow: the trigger already *is* an `AFTER`
trigger. The same trigger body attached as an `AFTER UPDATE` trigger on a
plain multi-row `UPDATE` — the exact pattern the `HINT` recommends — succeeds:
```sql
UPDATE ai2 SET name = name || '!';
UPDATE 2
id | name | touched
----+------+---------
1 | one! | 1
2 | two! | 1
```
A milder manifestation of the same root cause: an `AFTER INSERT` trigger on a
leftover can observe other rows targeted by the same statement still in their
pre-image state.
**Why it is not tested:** the fix is a design decision with no obvious
expected output — deferring the leftover triggers to end of statement would
change the per-leftover transition-table semantics that `8e72d91` and its
follow-ups deliberately introduced, so the "correct" result is exactly what
is in dispute. Pinning today's error would enshrine the bug.
### D8 — A `BEFORE INSERT` trigger returning `NULL` silently discards leftovers
Leftovers are inserted through the normal `ExecInsert()` path, so a
`BEFORE INSERT ... FOR EACH ROW` trigger that returns `NULL` suppresses them.
Nothing reports this, and the command tag still counts the row:
```sql
CREATE TRIGGER skip BEFORE INSERT ON t FOR EACH ROW
EXECUTE FUNCTION return_null();
UPDATE t FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01' SET name = 'x';
UPDATE 1
-- only the truncated [2002-01-01,2003-01-01) row survives; both leftovers,
-- i.e. all history outside the updated portion, are gone
DELETE FROM t FOR PORTION OF valid_at FROM '2004-01-01' TO '2005-01-01';
DELETE 1
-- the entire row is destroyed, not just the requested portion
```
So a trigger that a user installed to veto *new* rows also silently deletes
*existing* history.
**Why it is not tested:** this is arguably by design. The project has
already chosen, in commit `9170c8b`, to capture rather than reject a closely
analogous nonsensical outcome (a `BEFORE UPDATE` trigger that rewrites
`NEW.valid_at` during `FOR PORTION OF`), noting that DB2 rejects it while
MariaDB matches PostgreSQL. Adding a test that asserts the opposite
disposition would prejudge that call. It is, however, undocumented — neither
reference page mentions that a `BEFORE INSERT` trigger can suppress leftovers
— and it is not covered anywhere in `for_portion_of.sql`, which tests only a
tuple-*modifying* `BEFORE INSERT` trigger.
### D9 — Bounds are evaluated four times per row, at three independent sites
`transformForPortionOfClause()` deep-copies the bound expressions into
separate evaluation sites (the row-selection qual, the `SET`-list truncation
expression, and the executor's leftover computation). For a single row and a
single statement, each bound is evaluated four times:
```sql
UPDATE v FOR PORTION OF valid_at FROM nb('2002-01-01') TO nb('2003-01-01')
SET name = 'x';
NOTICE: bound evaluated -> 2002-01-01
NOTICE: bound evaluated -> 2003-01-01
NOTICE: bound evaluated -> 2002-01-01
NOTICE: bound evaluated -> 2003-01-01
NOTICE: bound evaluated -> 2002-01-01
NOTICE: bound evaluated -> 2003-01-01
NOTICE: bound evaluated -> 2002-01-01
NOTICE: bound evaluated -> 2003-01-01
```
Only `VOLATILE` functions are rejected, so a `STABLE`-labelled function whose
result is not actually stable yields *different* bounds at different sites
within one statement, producing overlapping history:
```sql
UPDATE v2 FOR PORTION OF valid_at FROM '2002-01-01' TO drift() SET name = 'x';
id | valid_at | name
----+-------------------------+------
1 | [2000-01-01,2002-01-01) | one
1 | [2002-01-01,2003-01-05) | x -- updated portion ends 01-05
1 | [2003-01-03,2010-01-01) | one -- leftover starts 01-03: overlap
```
On a table carrying a temporal primary key the overlap is caught
(`conflicting key value violates exclusion constraint`) and the statement
rolls back, so the invariant holds where one is declared; on an unconstrained
history table the corruption is silent. `DELETE` can likewise leave a gap.
**Why it is not tested:** triggering the divergence requires a function whose
declared volatility is a lie, and PostgreSQL's documented position is that
mislabelling volatility yields undefined results — so a committer could
reasonably decline to call the wrong-results half a bug. The evaluation
*count* is the objectively surprising part, but a test asserting "two
evaluations, not eight" would be pinning an implementation detail that
planner changes could legitimately alter.
### D10 — Plain `EXPLAIN` evaluates the `FOR PORTION OF` target expression
Same root cause as D3/D4. With a `STABLE` function in the target, plain
`EXPLAIN` runs it:
```sql
EXPLAIN (COSTS OFF) UPDATE t FOR PORTION OF valid_at (noisy()) SET name = 'q';
NOTICE: called
NOTICE: called
```
A function in the target of an `EXPLAIN`ed statement can therefore raise
errors, consume resources, or emit messages. It can even reject the
`EXPLAIN` outright: if the function body contains DML, `EXPLAIN` fails with
`INSERT is not allowed in a non-volatile function`.
**Why it is not tested:** exactly one of the two calls above is the defect.
The planner legitimately evaluates stable functions in a qual for selectivity
estimation, and the target expression is also copied into the `&&` qual, so
even correct behaviour emits one `NOTICE`. A regression test would have to
assert "one notice instead of two", which is fragile and would silently stop
testing anything if planner estimation changed. D4 exercises the same code
path deterministically and is tested instead.
### D11 — `WITH CHECK OPTION` blames the leftover, not the user's row
Leftovers are checked against the view's `WITH CHECK OPTION` before the row
the user's `SET` produced, so the `DETAIL` names a row the user never wrote,
showing its *pre-update* column values:
```sql
CREATE VIEW wv AS SELECT * FROM wt WHERE valid_at @> '2005-01-01'::date
WITH CHECK OPTION;
UPDATE wv FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01'
SET name = 'changed';
ERROR: new row violates check option for view "wv"
DETAIL: Failing row contains (1, [2000-01-01,2002-01-01), keepme).
```
The user's statement produced `(1, [2002-01-01,2003-01-01), changed)`, which
also violates the check option; the reported row is the untouched leftover.
**Why it is not tested:** the statement is correctly rejected and rolled
back, so only the error attribution is wrong; which of several violating rows
gets reported is not clearly specified behaviour.
### D12 — `NULL` bounds deparse as `NULL::unknown`
```sql
CREATE RULE r AS ON DELETE TO src DO INSTEAD
DELETE FROM t FOR PORTION OF valid_at FROM NULL TO '2001-01-01';
SELECT definition FROM pg_rules WHERE rulename = 'r';
... DELETE FROM t FOR PORTION OF valid_at FROM NULL::unknown TO '2001-01-01'
```
`FOR PORTION OF` retains the *uncoerced* bound expressions for deparsing, so
an untyped `NULL` literal is rendered with a cast to the `unknown`
pseudo-type. This reaches users through `pg_dump` and `\d+` output for any
rule whose action uses an unbounded `FOR PORTION OF`.
**Why it is not tested:** the output is merely ugly, not wrong — it re-parses
and restores correctly, so no user-visible failure results. In addition, any
plausible fix for D2 changes how these retained bound expressions are stored,
which would change this rendering as a side effect; pinning the current
spelling in an expected file would be counterproductive.
### D13 — `UPDATE`/`DELETE` reference pages omit `FOR PORTION OF` privileges
`doc/src/sgml/ref/update.sgml` states:
> You must have the `UPDATE` privilege on the table, or at least on the
> column(s) that are listed to be updated.
`FOR PORTION OF` silently adds the range column to the set of updated
columns (commit `7ac030d`), but the range column is not "listed to be
updated" by the user, so the requirement is undocumented:
```sql
GRANT SELECT (id, valid_at, name), UPDATE (name) ON t TO alice;
-- as alice:
UPDATE t FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01' SET name = 'x';
ERROR: permission denied for table t
```
`doc/src/sgml/ref/delete.sgml` has the mirror-image gap: it requires `SELECT`
only "for any table in the `USING` clause or whose values are read in the
condition", but `DELETE ... FOR PORTION OF` also requires `SELECT` on the
range column.
Both pages *do* document the more surprising direction (that the leftover
inserts need no `INSERT` privilege), which is correct — that claim was
verified.
**Why it is not tested:** the implemented behaviour is correct and already
has regression coverage; only the documentation is incomplete, and
documentation gaps cannot be expressed as a regression test.
### D14 — Misleading error through a view exposing the range column twice
```sql
CREATE VIEW v AS SELECT id, valid_at AS a, valid_at AS b, name FROM t;
UPDATE v FOR PORTION OF a FROM '2002-01-01' TO '2003-01-01'
SET b = daterange('1990-01-01','1991-01-01');
ERROR: multiple assignments to same column "valid_at"
```
The user assigned exactly one column, so the message is confusing. The check
that would give the right message —
`cannot update column "..." because it is used in FOR PORTION OF` — is applied
in `transformUpdateTargetList()` against the *view's* column names, so it does
not notice that `b` and the `FOR PORTION OF` column `a` are the same base
column; the collision is caught later, by the generic rewriter check.
**Why it is not tested:** the outcome is safe (the statement is rejected, no
data is corrupted) and the message wording is the only problem, so this is a
polish item rather than a defect worth pinning in an expected file.
---
## Root causes
**D1** — `RewriteQuery()` (`src/backend/rewrite/rewriteHandler.c`) deliberately
defers attaching the `FOR PORTION OF` qual and range target list when the
result relation is a view:
```c
if (rt_entry_relation->rd_rel->relkind != RELKIND_VIEW)
{
AddQual(parsetree, parsetree->forPortionOf->overlapsExpr);
...
}
```
The comment explains that this avoids adding the same qual twice "on the
recursion" — i.e. it assumes rewriting will recurse onto the same `Query`
once the view is expanded. A `DO INSTEAD` rule breaks that assumption: it
discards the `Query` carrying `forPortionOf` and substitutes the rule action,
which has no `forPortionOf` of its own. The clause therefore disappears
without ever being applied. The neighbouring guard for `INSTEAD OF`
triggers (`views with INSTEAD OF triggers do not support FOR PORTION OF`,
commit `dfce19c`) covers only the trigger form of the same hazard.
Note that `DO INSTEAD` rules on a *table* are unaffected in the same way and
are already covered by existing tests, because for non-views the qual is
attached before rules fire.
**D2, D12** — `transformForPortionOfClause()` (`src/backend/parser/analyze.c`)
builds the range constructor from *copies* of the bound expressions:
```c
args = list_make2(copyObject(result->targetFrom),
copyObject(result->targetTo));
...
make_fn_arguments(pstate, args, actual_arg_types, declared_arg_types);
```
`make_fn_arguments()` resolves an untyped `PARAM_EXTERN` through
`p_coerce_param_hook`, which records the resolved type *and* fixes the `Param`
node in place — but only for the copy. The originals kept in
`ForPortionOfExpr.targetFrom` / `.targetTo` (retained for deparsing) keep
`paramtype == UNKNOWNOID`, so `check_parameter_resolution_walker()` sees a
`Param` whose type disagrees with the resolved parameter list and raises
`could not determine data type of parameter $n`. Dropping the two
`copyObject()` calls makes D2 disappear; it also leaves every existing
expected-output file unchanged.
**D3, D4, D10** — `ExecInitModifyTable()`
(`src/backend/executor/nodeModifyTable.c`) evaluates the target
unconditionally at executor start-up:
```c
if (node->forPortionOf)
{
...
exprState = ExecPrepareExpr((Expr *) forPortionOf->targetRange, estate);
targetRange = ExecEvalExpr(exprState, econtext, &isNull);
if (isNull)
ereport(ERROR, ... "FOR PORTION OF target must not be null" ...);
```
There is no `EXEC_FLAG_EXPLAIN_ONLY` guard, even though the same function
already skips `ExecSetupTransitionCaptureState()` in explain-only mode a few
hundred lines earlier. Under `EXPLAIN` the expression is evaluated (D10), a
`Param` has no value yet under `GENERIC_PLAN` (D3), and the run-time null
check fires (D4). Adding `&& !(eflags & EXEC_FLAG_EXPLAIN_ONLY)` to the
condition fixes all three.
**D7** — `ExecForPortionOfLeftovers()` opens and closes a nested
after-trigger query context per leftover (`nodeModifyTable.c:1616-1621`).
All three candidate fixes were applied temporarily, and only temporarily, to
confirm the diagnoses and to generate the expected output for the new tests;
**the committed tree contains no source changes**, only new tests.
---
## Test status
The new tests encode *correct* behaviour, so they fail against `0b776de`;
the failure diff is the defect report. Run them with:
```
meson setup build -Dcassert=true -Dtap_tests=enabled && ninja -C build install
build/src/test/regress/pg_regress --bindir=<prefix>/bin \
--inputdir=src/test/regress --expecteddir=src/test/regress \
--dlpath=build/src/test/regress --temp-instance=/tmp/ti for_portion_of
```
On master this produces a 190-line, 5-hunk diff containing 11 unexpected
`ERROR` lines (D2/D3/D4) and 3 missing `ERROR` lines (D1), plus the wrong
row contents D1 leaves behind. All hunks fall inside the two new blocks,
which begin at line 2796 of the expected file; the 2795 lines of pre-existing
expected output are untouched. With the three candidate fixes applied the
test passes.
---
## Rejected candidate findings
Reported by hunt agents, but withdrawn after hands-on verification:
* *A `BEFORE UPDATE` trigger that rewrites `NEW.valid_at` produces senseless
results.* Already covered by an existing test
(`for_portion_of.sql:1228-1270`, commit `9170c8b`), which deliberately
captures the current behaviour.
* *`DELETE ... FOR PORTION OF` does not require `UPDATE` privilege on the
range column, unlike `UPDATE`.* Intentional and already enshrined in
`src/test/regress/sql/privileges.sql`.
* *RLS `INSERT` policies on inheritance children / partition leaves are
skipped for leftovers.* Plain non-temporal `INSERT` behaves identically;
a pre-existing general limitation, not caused by `8e72d91`.
* *Rule `DO ALSO` side effects are invisible in the command tag*, and
*`RETURNING OLD`/`NEW` collide with rule `OLD`/`NEW` pseudo-relations.*
Both reproduce without `FOR PORTION OF`.
---
## Areas examined without finding a defect
For coverage accounting, the following were probed empirically against the
cassert build and behaved correctly:
* Range algebra: ~700 combinations of row range × target bounds for
`int4range` (inclusive/exclusive/infinite/empty/`NULL` bounds), compared
against `range_intersect` / `multirange_minus` ground truth — no mismatch.
* Leftover count and content for range and multirange columns, including the
documented "multiranges never need two leftovers" claim.
* `RETURNING OLD.*` / `NEW.*` for both `UPDATE` and `DELETE`; leftovers
correctly excluded from `RETURNING` and from the command tag; row counts and
`GET DIAGNOSTICS ROW_COUNT` across ~20 shapes.
* Transition tables (`REFERENCING OLD/NEW TABLE`) for statement- and
row-level triggers, including partitioned and cross-partition cases.
* Partitioning: cross-partition moves, sub-partitioning, `DEFAULT`
partitions, leftovers routed to a partition pruned from the plan, foreign
table partitions, and prepared plans surviving detach/reattach.
* Plain inheritance leftover routing (multi-level, dropped/reordered columns,
child-local indexes) — correct since `7d13b03`; only the statement-trigger
target relation is wrong (D6).
* Views: auto-updatable, subset-column, renamed-column, nested (1–3 levels),
`security_barrier`, and `WITH CHECK OPTION` (`LOCAL` and `CASCADED`);
deparse round-trip through `pg_get_ruledef` including quoted and Unicode
identifiers and column renames.
* Constraints: table `CHECK`, domain `CHECK`, domains over ranges and
multiranges, temporal primary keys (`WITHOUT OVERLAPS`), temporal foreign
keys (`PERIOD`) in both directions, `EXCLUDE` constraints, including
deferred forms.
* Identity and `DEFAULT` columns, generated columns (`STORED` and `VIRTUAL`),
dropped columns, `TOAST`ed values (each leftover receives its own TOAST
value — no pointer sharing, so no reclamation hazard).
* Privileges and RLS, including the documented "leftover inserts need no
`INSERT` privilege" behaviour and `FORCE ROW LEVEL SECURITY`.
* Bound expressions: column references, subqueries, aggregates, window
functions, set-returning functions, `DEFAULT`, and volatile functions are
all rejected with correct error codes and cursor positions.
* Stored `NULL` and empty ranges: the overlap qual `range && target` is ANDed
onto the statement's `WHERE`, and yields `NULL`/`false` for such rows, so
they are never touched and the defensive `found a NULL range in a temporal
table` `elog` is unreachable from ordinary SQL.
* Concurrency: `READ COMMITTED` `EvalPlanQual` re-checks racing a concurrent
range-shrinking `UPDATE`, including the trigger-driven EPQ path.
* Plan caching: generic plans over seven executions, and plan invalidation by
`ALTER TABLE ... ALTER COLUMN TYPE` / `DROP COLUMN` underneath a prepared
`FOR PORTION OF` statement.
* Documentation claims in `update.sgml`, `delete.sgml`, `dml.sgml`,
`trigger.sgml` and the glossary, executed verbatim.
* Miscellaneous: data-modifying CTEs, `COPY (... FOR PORTION OF ...
RETURNING) TO`, `debug_parallel_query = on`, `UPDATE ... FROM` with a
self-join matching the target row twice, and `WHERE CURRENT OF` rejection.
