Technical Analysis: Null Pointer Dereference in FOR PORTION OF with Views and INSTEAD OF Triggers

Core Problem

A null pointer dereference crash occurs in PostgreSQL 19 when combining three features: views, the FOR PORTION OF syntax (a temporal table feature), and INSTEAD OF triggers. The crash manifests in ExecModifyTable() around line 4827, where the code path for FOR PORTION OF processing does not account for the case where the target relation is a view (RELKIND_VIEW).

Root Cause Mechanics

When a DML operation targets a view with an INSTEAD OF trigger, the executor does not have a physical tuple identifier (tupleid) because views don't store tuples — the trigger handles the actual modification. However, the FOR PORTION OF machinery (specifically ExecForPortionOfLeftovers(), called from ExecDeleteEpilogue() or ExecUpdateEpilogue()) unconditionally dereferences tupleid, which is NULL in the view case. This is a straightforward oversight in the interaction between the relatively new FOR PORTION OF feature and the existing INSTEAD OF trigger code path.

Architectural Issues Identified

Tom Lane's review exposed much deeper architectural concerns beyond the immediate crash fix. The FOR PORTION OF implementation (originally committed as part of a larger temporal tables feature) has several parse-time checks that violate PostgreSQL's phase separation principles:

1. Premature View/FDW Checks at Parse Time

The original code in transformForPortionOfClause checks relkind at parse time. This is architecturally wrong because:

• A view could gain or lose INSTEAD OF triggers between parse time and execution time
• Parse trees can be stored in rules, new-style SQL functions, or prepared statements
• The state of the world at execution time may differ from parse time

2. Premature Volatility Check

contain_volatile_functions_after_planning is called at parse time, but this function is designed for post-planning use (as its name implies). It should be called in the planner where constant folding has already occurred, making it both more correct and cheaper.

3. FDW Partition Coverage Gap

The anti-FDW check at parse time cannot detect the case where a partitioned table has foreign data wrapper partitions as children. This requires runtime checking to properly handle partition routing.

4. Opclass Lookup Timing (Partially Retracted)

Tom initially flagged the opclass lookups as premature, but later partially retracted this concern. The analogy to IN-clause equality operator resolution makes the parse-time opclass resolution acceptable — the dependency is on specific operators/functions, not on the opclass structure itself. However, he noted that dependencies must be properly recorded, and that dependency tracking only guarantees existence, not property stability.

Proposed Solutions

Immediate Fix (v4/v5 by Paul Jungwirth)

• Skip all FOR PORTION OF processing when an INSTEAD OF trigger fires on a view
• Document that FOR PORTION OF work is bypassed in this case
• This addresses the crash without restructuring the feature

Structural Fix (jian he's approach)

• Move the validity check into CheckValidResultRel (an existing executor function that validates target relations)
• Pass the full ModifyTable node to CheckValidResultRel instead of adding another sub-node check — described as "more future-proof"
• Move contain_volatile_functions_after_planning to ExecInitModifyTable

Volatility Check Placement

jian he analyzed the ForPortionOfExpr structure and determined:

• targetFrom/targetTo are only for deparsing — no need to process
• rangeTargetList and overlapsExpr already go through eval_const_expressions during rewriting
• Only ForPortionOfExpr->targetRange needs explicit handling
• ExecInitModifyTable already calls ExecPrepareExpr on targetRange, which internally does eval_const_expressions
• Cannot use ExprState->expr for the volatility check because that field is "for debugging only" per execnodes.h comments
• The volatility check should be done on the original node before ExecPrepareExpr

Final Combined Approach

Paul Jungwirth created a new thread with a commitfest entry combining:

1. jian he's CheckValidResultRel approach for runtime validation (preferred over Paul's own patch)
2. Paul's test additions including a case for pruned FDW partitions (ensuring no false-positive errors when a partition child FDW is pruned away)
3. Moving the volatility check to plan-time

Key Design Tradeoffs

1. Parse-time vs. execution-time checks: PostgreSQL's stored parse trees (in rules, SQL functions) mean parse-time assumptions about relkind, triggers, and catalog state are fragile. The principle is: anything that could change must be checked at execution time.

2. Error granularity vs. performance: Moving checks to execution time means some errors that could be caught early are deferred. However, correctness trumps early error reporting.

3. Future-proofing CheckValidResultRel: Passing the full ModifyTable node rather than individual fields anticipates that other execution-time checks may need access to more plan information.