Add sanity checks for ENR entries.

[timchang514]

Description

Tests are running in babelfish-for-postgresql/babelfish_extensions#3245

This change adds a number of sanity checks when ENR entries are added. Previously, there were no checks here which meant that we could run into cases we had not considered where an ENR entry could depend on a non-ENR entry, and vice-versa. When adding these tests, I discovered that there are still 3 on-disk catalogs that can be referenced by ENR entries:

• pg_collation
• pg_authid
• pg_opclass

These catalogs can't be trivially added to ENR, so I will create some followup tasks on properly handling these cases if possible.

Note that there are still some potential limitations. For example, the issue fixed in BABEL-4868 (see babelfish-for-postgresql/babelfish_extensions#3122) would not have been caught by this change due to the way that dependency entries are created for functions as column defaults.

A way to truly guarantee that all corner cases could be caught is to implement a sort of check in Postgres' dependency code. This would be a pretty sizable change and have potential performance impact, since we would need to a) add infrastructure to allow lookups to all tuples in ENR by OID, and b) search through this structure every time we try to add a dependency. I've opted to not include that for now due to the reasons mentioned, but if there is interest in that we can have a separate discussion on it.

Issues Resolved

BABEL-4776

Check List

• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the PostgreSQL license, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[surendravishnoi]

this adds value but will not detect cases where there is dependency between temp and main objects

[timchang514]

see above comment.

[surendravishnoi]

what value these conditions add? for example temp tables cannot have row level security attributes. But what we really want to check is that any part of an ENR table should not be stored in PG catalog. It means that I cannot have an entry in PG catalog to store row level security information. Either I store that in ENR or whole table goes into PG catalog. But are these checks right candidates to validate above assumptions?

[timchang514]

It's at least some amount of validation on the pg_class entry. You're correct that this doesn't check whether there is an entry for an ENR table in the PG catalog for row level security information. But that row is still tied to a pg_class entry with the flag set, so this will still catch that case and throw an error.

As I mentioned in the description of the PR:

Note that there are still some potential limitations. For example, the issue fixed in BABEL-4868 (see babelfish-for-postgresql/babelfish_extensions#3122) would not have been caught by this change due to the way that dependency entries are created for functions as column defaults.

A way to truly guarantee that all corner cases could be caught is to implement a sort of check in Postgres' dependency code. This would be a pretty sizable change and have potential performance impact, since we would need to a) add infrastructure to allow lookups to all tuples in ENR by OID, and b) search through this structure every time we try to add a dependency. I've opted to not include that for now due to the reasons mentioned, but if there is interest in that we can have a separate discussion on it.

[timchang514]

I will start a followup discussion re: the incompatible catalogs as well as the problems with checking for ENR entries in pg catalogs. I wouldn't consider either of those blockers for this change.

[lejaokri]

I dont have any more comments