hi, one minor issue. not that minor, since many DDLs need to consider the system attribute.
looking at these functions: SearchSysCacheCopyAttName SearchSysCacheAttName get_attnum get_attnum says: Returns InvalidAttrNumber if the attr doesn't exist (or is dropped). So I conclude that "attnum == 0" is not related to the idea of a system column. for example, ATExecColumnDefault, following code snippet, the second ereport should be "if (attnum < 0)" ========== attnum = get_attnum(RelationGetRelid(rel), colName); if (attnum == InvalidAttrNumber) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" of relation \"%s\" does not exist", colName, RelationGetRelationName(rel)))); /* Prevent them from altering a system attribute */ if (attnum <= 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter system column \"%s\"", colName))); ========== but there are many occurrences of "attnum <= 0". I am sure tablecmds.c, we can change to "attnum < 0". not that sure with other places. In some places in tablecmd.c, we already use "attnum < 0" to represent the system attribute. so it's kind of inconsistent already. Should we do the change?