[OffgridwithJD]

The drop half of #1297, shape b from my design note, which you confirmed on #1297. Your six review points are covered below.

What it is

pgcolumnar.partition_drop_plan(parent, cutoff) returns the statements that retire every partition whose whole range is older than the cutoff, meaning its upper bound is at or before it. It returns them in range order and schema-qualified:

• A normal partition: ALTER TABLE … DETACH PARTITION … CONCURRENTLY, then DROP TABLE.
• A pending partition, left by an interrupted concurrent detach: DETACH PARTITION … FINALIZE, then the drop.
• A tiered partition (1.0: tier cold partitions to Parquet on object storage #1295): DROP FOREIGN TABLE. The Parquet file stays, and the docs say so.

It runs nothing, because DETACH … CONCURRENTLY refuses to run inside any function. You run the result with \gexec:

SELECT * FROM pgcolumnar.partition_drop_plan('metrics', now() - interval '90 days') \gexec

The parent is never locked exclusively. There is no ownership check, as you said: DETACH checks when the statements run.

Your six points

1. The cutoff's type is the key's. There are two overloads, timestamptz and timestamp. A key of the other type is refused with 42804, and the hint names the right expression. An untyped literal resolves to timestamptz. Arms:
   • a timestamp key with a timestamp cutoff gives the same plan from Los Angeles and Kolkata (Tokyo and Denver in the twin);
   • the refusal, both ways.
2. Tiered partitions get DROP FOREIGN TABLE. Arms: it is planned that way, and running it leaves the Parquet file.
3. MINVALUE is listed once its upper bound is past, and MAXVALUE is never listed. One arm covers both.
4. Quoting: a schema "Drop Zone" and partitions "Old Part"/"New Part" ("Py Drop", "Week One" in the twin). The arm runs the plan and requires every statement to succeed. My first version only checked which tables were left, and the unquoted-names mutation survived it: the DETACH failed, but the DROP then removed the still-attached partition, under ACCESS EXCLUSIVE on the parent. With the stronger arm, that mutation is red.
5. A sub-partitioned partition is listed whole, and the DROP takes its children. The arms check the plan, and that the children are gone after running it.
6. The storage-rows check is an arm, dated by a mutation that skips the drop hook's pgcolumnar_delete_storage_tree.

FINALIZE on all five majors: the pending-detach arm is green on 15–19 in both twins.

Evidence

• partition_lifecycle.sh gives 49/49 and test_partition_lifecycle.py 2/2 on each of PG15–19, then again on PG17 on this tree (main at 80bce53). compare_to_bash finds nothing missing.
• Red first: the original arms gave 42883, because the function was absent. The review arms were red on the first version: a cutoff of the other type was planned, not refused, and a tiered partition got DROP TABLE. The twin was also red against main.
• Sixteen mutations, each red on its own arms:
  • a pending detach not finalised;
  • a strict cutoff;
  • the type check removed;
  • the timestamp overload comparing across zones;
  • the DEFAULT refusal removed;
  • the key-type refusal removed;
  • catalog order instead of range order (the fixtures create partitions out of order);
  • no DROP;
  • a plain DETACH;
  • no cutoff filter;
  • the lower bound read for the upper;
  • MINVALUE left out;
  • a tiered partition dropped as a table;
  • unquoted names;
  • sub-partitions left out;
  • the drop hook skipped.
• One equivalent mutation: casting a timestamp bound to timestamptz reads it in the session's zone, so that cast is the wall-clock reading itself.
• Ledger: 22 rows, 18 dated, 4 never (all premises). 1801 → 1805, the gate gives rc=0, and suites_not_covered stays 246. One check, "a second plan for the same cutoff is empty", held under every mutation, so it is now a premise in both twins.
• PG17 neighbours:
  • native_upgrade_converge, entry_point_privilege, server_file_privilege, tier_partition and docs_style all pass;
  • docs_style caught two long sentences in the how-to, which are fixed.
• The guard half passes 419/419, and cluster_tests goes 526 → 527. The alpha5 install script and the alpha4 upgrade script carry identical patches.

Not done: the daemon running this (shape a). That still waits on the question from my design note.

🤖 Generated with Claude Code

[jdatcmd]

Review at b4a2386. The function is right as far as I read it: tiered partitions are detached first too, names come out schema-qualified through regclass under the function's search_path, the upper_bound::timestamptz cast reads an offset-bearing bound so no zone enters, and every one of my six points has an arm. One change I'd want before merge, and one small label.

A DROP after a failed DETACH takes the parent's exclusive lock

Your point 4 found it: when the DETACH failed, the following DROP removed the still-attached partition under ACCESS EXCLUSIVE on the parent. The stronger quoting arm closes that cause, but not the failure mode. Any DETACH can fail: a lock_timeout, a cancelled CONCURRENTLY wait (which also leaves the partition pending), or a permission error. \gexec carries on to the next statement unless ON_ERROR_STOP is set, and the how-to doesn't set it, nor would most schedulers. The DROP then blocks every query on the table and drops a partition the plan said it would detach first. The docs promise the opposite ("takes no exclusive lock on the parent").

What I'd do: make the drop statement refuse when the partition is still attached, so the plan is safe whatever runs it. For example, emit

DO $d$ BEGIN
  IF EXISTS (SELECT 1 FROM pg_inherits WHERE inhrelid = '<part>'::regclass) THEN
    RAISE EXCEPTION '<part> is still attached; its DETACH did not finish' USING ERRCODE = '55000';
  END IF;
  DROP TABLE <part>;
END $d$

A DROP inside DO is fine; only the DETACH ... CONCURRENTLY must be top-level. A pending partition is still in pg_inherits, so the DROP refuses that too, and the next plan's FINALIZE picks it up. Also put \set ON_ERROR_STOP on in the how-to example, as a second line of defence.

Arm, in both twins: make a DETACH fail (lock_timeout while another session holds a lock on the parent), run the rest of the plan, and require the partition to still exist and still be attached, and the DROP to fail with 55000.

Small

"premise: a second plan for the same cutoff is empty" is behaviour, not a fixture fact. A plan that kept listing dropped or pending partitions would redden it, so under #1236 it is an arm. It probably needs no new mutation: one that drops the upper_bound <= cutoff filter, or keeps detached partitions in the candidates, should redden it.

Checked

• Ledger: main's 2286 rows are byte-identical outside partition_lifecycle. 2308 rows in all, of which 1805 are never red, equal to the budget. All ten never-red partition_lifecycle rows are labelled premises; the one above is the only one I'd move.
• CI: still running at the time of writing.

[jdatcmd]

@OffgridwithJD, the owner has decided that alpha5 waits for this PR. All seven time-series features are merged (#1292 last, via #1330), so this is now the last item before the tag. The release-notes PR is #1333. When you push the fix for my review (the DROP refusing while the partition is still attached, and the premise that should be an arm), I'll review it straight away. I'll also add a line about partition_drop_plan to #1333.

[OffgridwithJD]

@jdatcmd Thanks: that was the real failure mode behind my quoting finding. Both points are addressed at d7effdc, rebased onto b55cb23 (#1330).

The guarded drop. Each partition's drop is now:

DO $pgcolumnar_drop$ BEGIN IF EXISTS (SELECT 1 FROM pg_catalog.pg_inherits WHERE inhrelid = '<part>'::regclass)
  THEN RAISE EXCEPTION '% is still attached to its parent; its DETACH did not finish', '<part>' USING ERRCODE = '55000';
  END IF; DROP TABLE <part>; END $pgcolumnar_drop$

It is one line, with the names through %L and regclass output, and DROP FOREIGN TABLE for a tiered partition. A pending partition is still in pg_inherits, so its drop refuses too, and the next plan finalises it. The how-to example now sets \set ON_ERROR_STOP on, and both the how-to and the reference say what the refusal guards.

The arm, in both twins. Another session holds SHARE UPDATE EXCLUSIVE on the parent, and the plan's DETACH runs with lock_timeout and fails (55P03). The holder then lets go, and the DROP runs with nothing in its way but its own guard. It must refuse (55000), and the partition must still exist and be attached.

• Red first: the plain DROP succeeded (55P03;ok) and the partition was gone.
• Green now: 55P03;55000|2|1.

My first attempt kept the holder's lock through the DROP, so the DROP also timed out and the arm was red for the wrong reason. The drop now runs after the lock is released.

"a second plan … is empty" is an arm again: "a second plan for the same cutoff is empty, and the partitions it kept are still attached". The second half is what a mutation reaches. Without the cutoff filter, the first run drops everything, and the second plan was empty anyway.

Evidence:

• Matrix: partition_lifecycle.sh 50/50 and the twin 2/2 on each of PG15–19.
• Mutations: seventeen, each red on its arms. The new one is unguarded_drop, a plain DROP: red at the failed-DETACH arm and at every plan-text arm, and the twin is red under it too.
• Ledger: 23 rows, 20 dated, 3 never (all premises). 1801 → 1804, and the gate gives rc=0.
• PG17 neighbours: native_upgrade_converge, entry_point_privilege, server_file_privilege, docs_style and tier_partition all pass.
• Guard half: 419/419, and cluster_tests goes 527 → 528.

How it was rebased. I rebuilt the branch by cherry-picking onto main without my superseded ledger commit, then regenerated the ledger from main's rows. The per-file patches match the pre-rebuild branch, except expected_tests, which moved from 527 to 528.

[jdatcmd]

Approved at d7effdc. Merging when CI is green on this head; alpha5 waits for this PR.

What I checked:

• The guarded drop: the plan text, %L and regclass for every name, DROP FOREIGN TABLE for a tiered partition, and a pending partition refused by the same pg_inherits test.
• The second-plan arm: it reaches a mutation now, through "the partitions it kept are still attached".
• Ledger: main's 2286 rows are byte-identical outside partition_lifecycle. 2309 rows, of which 1804 are never red, equal to the budget, and every never-red row in the suite is a premise.
• PG19, which CI doesn't build: partition_lifecycle.sh 50/50 at this head.

A nit, not blocking. A partition name can contain $pgcolumnar_drop$, which ends the DO block's dollar quote early. I probed it on PG17 with two crafted names, and it can't be exploited. The text before the name always leaves the DO body inside an unfinished string literal, because %L doubles any quote the name carries. So the DO fails to compile, and that aborts the rest of the query string. The injected CREATE TABLE never ran, and the 63-byte name limit leaves little room besides. Still, format('DO %L', body) would make the body a proper literal and remove the question. Worth a follow-up whenever convenient.

[OffgridwithJD]

@jdatcmd Thanks. I'll take the dollar-quote nit as a follow-up PR after this merges, so alpha5 isn't held for it. It will emit DO with the body as a %L literal, and the arm will be red first: a partition whose name contains $pgcolumnar_drop$ must get a plan that runs as returned.

[jdatcmd]

Merging: approved at d7effdc, and CI is green on that head.