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 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. 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 change felt 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 a known pgindent issue that I reported before. With this patch applied, all regression tests pass. [1] https://postgr.es/m/caeowx2kggo1n2kdh6osfxhl_5gkg3dqq0pdnul4lh4xstkj...@mail.gmail.com Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v1-0001-tablecmds-cleanup-unreachable-AT_AddIndexConstrai.patch
Description: Binary data
