On Mon, Mar 16, 2026 at 05:07:51PM +0800, Chao Li wrote: > Basically, 0002 does the same thing as 0001 just on a different > sub-command of ALTER TABLE.
case AT_DropInherit: /* NO INHERIT */
[...]
- /* No command-specific prep needed */
+ ATPrepChangeInherit(rel);
This change means that we are plugging in earlier a check based on a
typed table for the NO INHERIT case. This sequence fails already on
HEAD and with the patch, but generates a different error in the last
query between HEAD and the patch, and is not covered by your patch:
CREATE TYPE person_type AS (id int, name text);
CREATE TABLE persons OF person_type;
CREATE TABLE stuff (a int);
ALTER TABLE persons NO INHERIT stuff;
I'd suggest the addition of a test in typed_table.sql, just after the
"ALTER TABLE persons INHERIT stuff;". The INHERIT case is already
blocked, so NO INHERIT is a no-op anyway because we complain about the
typed table not being inherited. How about doing that as a separate
patch, with the second patch for tablescmds.c updating the regression
test output? I thought that the NO INHERIT command was allowed, but
we fail, so blocking it with a different, somewhat clearer error is OK
by me.
You are right that the comment on top of ATPrepAddInherit() is wrong,
and that we'd better clean it up. The code does not do what the
comment tells. That does not seem worth troubling stable branches.
A second thing is that we'd better add an assertion in
ATExecDropInherit() to make sure that this code path is never taken
for a RELKIND_PARTITIONED_TABLE, ensuring that the prep phase blocked
this case?
--
Michael
signature.asc
Description: PGP signature
