Re: Odd pg dump error: cache lookup failure

On 2020-Oct-22, Tom Lane wrote:

> 1. I think this bit in LockViewRecurse_walker must be removed as well:
> 
>             /* Currently, we only allow plain tables or views to be locked. */
>             if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE &&
>                 relkind != RELKIND_VIEW)
>                 continue;
> 
> (Mostly, stuff in views would be ordinary tables anyway ... but
> foreign tables are a possible exception.)

Actually, foreign tables are not a problem: I guess they were hammered
out enough when somebody allowed them to be partitions, for partitioned
table recursive locking.  What *is* a problem with that change is any
RTE that's not RTE_RELATION.  It's not obvious in the above test, but
the most important case being left out is "relkind == '\0'" which is
what you get for other RTEs; so that broke when removing the above.  So
I added a "continue" for other RTE kinds, and now it all seems to work
correctly as far as I can tell.

While on this: it's important to note that lack of recursion here to
join RTEs does not break anything, since the relations being joined are
separately part of the rtable so they'll be processed anyway.

BTW I don't know what's the possible usefulness of locking standalone
composite types, but it's there.

> 2. Should we add anything to the LOCK TABLE reference page?  I see nothing
> there now that bears on this change, but ...

Eh.  There's a couple of places where it now seems more correct to talk
about a *relation* being locked.  I went as far as mentioning the set of
relkinds, for extra clarity (I skipped toast tables because it seemed
pointless).  The subject of the ONLY keyword not applying to views was a
bit surprising so I added that too.  Not 100% wedded on the wording of
these changes.

> 3. Is hard-coding #define's for ERRCODEs actually the best we can do here?
> I guess it is right now, but ugh.  Seems like we ought to find a way for
> frontend code to make use of errcodes.h.  Not a matter for this patch
> though.

Yeah, I had the same though -- didn't like this at all but evidently it
hasn't bothered anyone sufficiently.

> 4. You neglected to change the table name in the warn_or_exit_horribly
> error message.

Ugh, fixed.

> There might be some value in using one of pg_class's indexes instead,
> mainly because the parser will surely need to touch that on the way
> to performing the LOCK, while pg_aggregate is far afield.

Hmm.  I ended up using pg_class_tblspc_relfilenode_index, which I
suppose would be the least trafficked index of pg_class.  At least,
holding that one doesn't block other connections (so I could use
LOCK TABLE <that index>;
in another session and still launch pg_dump to verify it worked correctly)