Are we using FirstNormalObjectId correctly?

[Ning Yu]

Hi hackers,

While reading the source code I notice that we use FirstNormalObjectId to check whether a relation is a catalog table, for example in https://github.com/greenplum-db/gpdb/commit/3fe43b8a6785916fec018ee08ea38de7c4bd14e6 there are logics like `relid > FirstNormalObjectId`.  However according to the comments in "catalog/pg_magic_oid.h":

```

/* ----------

 * Object ID (OID) zero is InvalidOid.

 *

 * OIDs 1-9999 are reserved for manual assignment (see the files

 * in src/include/catalog/).

 *

 * OIDS 10000-16383 are reserved for assignment during initdb

 * using the OID generator.  (We start the generator at 10000.)

 *

 * OIDs beginning at 16384 are assigned from the OID generator

 * during normal multiuser operation.  (We force the generator up to

 * 16384 as soon as we are in normal operation.)

 *

 * The choices of 10000 and 16384 are completely arbitrary, and can be moved

 * if we run low on OIDs in either category.  Changing the macros below

 * should be sufficient to do this.

 *

 * NOTE: if the OID generator wraps around, we skip over OIDs 0-16383

 * and resume with 16384.  This minimizes the odds of OID conflict, by not

 * reassigning OIDs that might have been assigned during initdb.

 * ----------

 */

```

Looks like we should use `>=` instead of `>` in the comparison.  In postgres a similar check is in `json_categorize_type()`: `typoid >= FirstNormalObjectId`.  So I wonder whether we are incorrectly using this magic in gpdb.

A discussion related to that commit: https://groups.google.com/a/greenplum.org/d/msg/gpdb-dev/PC0Z_zw-YaY/tsCmPdADBQAJ

Best Regards

Ning

[Jimmy Yih]

I was wondering about this too a while back.  I agree that it should probably be >= instead of >.

Regards,

Jimmy

[Richard Guo]

As I can see, PostgreSQL always uses >= or < when related to FirstNormalObjectId.

I think it should be >= in these two places.

Thanks

Richard

[Daniel Gustafsson]

> On 15 Mar 2019, at 02:15, Ning Yu <n...@pivotal.io> wrote:

> Looks like we should use `>=` instead of `>` in the comparison. In postgres a similar check is in `json_categorize_type()`: `typoid >= FirstNormalObjectId`. So I wonder whether we are incorrectly using this magic in gpdb.

Correct, it should always be >= FirstNormalObjectId in all cases when testing
for user objects. Can you open a PR for this?

cheers ./daniel

[Ning Yu]

Thank you guys for the information, in such a case I'll open a PR for it.

[Ning Yu]

PR https://github.com/greenplum-db/gpdb/pull/7187 was opened and merged to fix the comparisons.