Hi Hackers,
While working on the patch [1], I was looking at the handling of AT_AddIndexConstraint in ATPrepCmd():
```
ATPrepCmd()
{
case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_PARTITIONED_TABLE);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_ADD_INDEXCONSTR;
}
```
I initially thought the comment about “never recurses” was stale, but after some debugging, I found that this branch is
actuallyunreachable. 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
isunreachable. I did think about just deleting the branch, but decided to keep it: if it were removed entirely, readers
mightwonder why AT_AddIndexConstraint is not handled in ATPrepCmd() and end up spending time debugging this themselves.
Two things to note in the patch:
1) The test edits are purely cosmetic. They just change lowercase keywords to uppercase to align with surrounding SQL
statements,and remove an unnecessary whitespace. I noticed this while debugging ADD CONSTRAINT USING INDEX, and the
changefelt too trivial to file a separate patch.
2) There is an unnecessary empty line after "case AT_AddIndexConstraint:" that was added by pgindent. I believe this is
aknown pgindent issue that I reported before.
With this patch applied, all regression tests pass.
[1] https://postgr.es/m/CAEoWx2kggo1N2kDH6OSfXHL_5gKg3DqQ0PdNuL4LH4XSTKJ3-g@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
