Re: BUG #19099: Conditional DELETE from partitioned table with non-updatable partition raises internal error

On Thu, Oct 30, 2025 at 10:48 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Kirill Reshke <reshkekirill(at)gmail(dot)com> writes:
> > Tom wrote:
> >> It's surely pretty accidental (and arguably not desirable)
> >> if "DELETE FROM pt WHERE false" doesn't fail the same way.
>
> > I cannot prove to myself why failing here is actually desirable. Can
> > you elaborate?
>
> If we throw that failure in some cases but not others, we're exposing
> implementation details.
>
> The definition could have been "throw 'cannot delete from foreign
> table' only if the query actually attempts to delete some specific
> row from some foreign table", but it is not implemented that way.
> Instead the error is thrown during query startup if the query has
> a foreign table as a potential delete target. Thus, as things stand
> today, you might or might not get the error depending on whether
> the planner can prove that that partition won't be deleted from.
> This is not a great user experience, because we don't (and won't)
> make any hard promises about how smart the planner is.
>
> An analogy perhaps is that whether you get a "permission denied"
> error about some target table is not conditional on whether the
> query actually attempts to delete any rows from it. We go out
> of our way to make sure that that happens when required by spec,
> even if the planner is able to prove that no delete will happen.
>
> None of this is meant to justify throwing an internal error here;
> that's clearly bad. I'm just saying that there would be little
> wrong with fixing it by throwing "cannot delete" instead. The user
> has no right to expect that that won't happen in a case like this.

We might be able to throw the "cannot delete from foreign table" like this:

@@ -987,6 +987,16 @@ add_row_identity_columns(PlannerInfo *root, Index rtindex,

fdwroutine = GetFdwRoutineForRelation(target_relation, false);

+ if (fdwroutine->ExecForeignDelete == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot delete from foreign table \"%s\"",
+ RelationGetRelationName(target_relation))));
+ if (fdwroutine->ExecForeignUpdate == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot update foreign table \"%s\"",
+ RelationGetRelationName(target_relation))));
if (fdwroutine->AddForeignUpdateTargets != NULL)
fdwroutine->AddForeignUpdateTargets(root, rtindex,
target_rte, target_relation);

but I am not sure how consistent the following is after applying that:

postgres=# set enable_partition_pruning to off;
SET
postgres=# EXPLAIN verbose DELETE FROM pt WHERE false;
ERROR: cannot delete from foreign table "p1"
postgres=# set enable_partition_pruning to on;
SET

-- we don't even hit the foreign table in the planner
postgres=# EXPLAIN verbose DELETE FROM pt WHERE false;
QUERY PLAN
-------------------------------------------------------
Delete on public.pt (cost=0.00..0.00 rows=0 width=0)
-> Result (cost=0.00..0.00 rows=0 width=0)
Output: ctid
Replaces: Scan on pt
One-Time Filter: false
(5 rows)

--
Thanks, Amit Langote