Re: ATPrepCmd: cleanup unreachable AT_AddIndexConstraint handling

> On Jan 29, 2026, at 13:12, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:
>> On Jan 28, 2026, at 14:14, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>> I initially thought the comment about “never recurses” was stale, but after some debugging, I found that this branch is actually unreachable. So leaving the code and comments in an unreachable branch would be confusing for readers.
>>>
>>> This patch cleans up the handling by putting an Assert(false) there and adding a comment to explain why this code path is unreachable. I did think about just deleting the branch, but decided to keep it: if it were removed entirely, readers might wonder why AT_AddIndexConstraint is not handled in ATPrepCmd() and end up spending time debugging this themselves.
>
>> I thought over and decided to delete AT_AddIndexConstraint from ATPrepCmd, which should be cleaner.
>
> Your first version was very substantially better. The Assert is
> important to help debug things if somebody changes the parsing
> logic in a way that falsifies the assumption that we can't get
> here for AT_AddIndexConstraint. And, as you thought originally,
> it's better to clearly document why we think this case is
> unreachable than to leave it looking like possibly an oversight.
> (I do not think a comment in some other case-branch accomplishes
> that.)
>
> Also, a look at the code coverage report suggests that the same
> might be true for AT_AddIndex. Can we replace that branch too
> with an Assert(false)?
>
> Matter of taste perhaps, but if I were committing this I would
> drop these case-folding-only changes in the regression tests.
> That's just useless code churn, accomplishing nothing except
> to create a hazard for possible future back-patches.
>
> regards, tom lane

Thanks for the guidance.

I verified AT_AddIndex with:
```
create table t1 (id int);
alter table t1 add primary key (id);
```

“Add primary key” is also initially parsed as AT_AddConstraint, then transformed to AT_AddIndex during the execution phase.

So in v3 I reverted back to the v1 approach and placed AT_AddIndex next to AT_AddIndexConstraint in ATPrepCmd, putting them in the same branch so they share the same assertion and explanatory comment.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/