change "attnum <=0" to "attnum <0" for better reflect system attribute - Mailing list pgsql-hackers

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?