LargeObjectRelationId vs LargeObjectMetadataRelationId, redux

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Subject: LargeObjectRelationId vs LargeObjectMetadataRelationId, redux
Date: 2023-12-13 19:53:29
Message-ID: 2650449.1702497209@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

By chance I discovered that checks on large object ownership
are broken in v16+. For example,

regression=# create user alice;
CREATE ROLE
regression=# \c - alice
You are now connected to database "regression" as user "alice".
regression=> \lo_import test
lo_import 40378
regression=> comment on large object 40378 is 'test';
ERROR: unrecognized class ID: 2613

This has been failing since commit afbfc0298, which refactored
ownership checks, replacing pg_largeobject_ownercheck and
allied routines with object_ownercheck. That function lacks
the little dance that's been stuck into assorted crannies:

if (classId == LargeObjectRelationId)
classId = LargeObjectMetadataRelationId;

which translates from the object-address representation with
classId LargeObjectRelationId to the catalog we actually need
to look at.

The proximate cause of the failure is in get_object_property_data,
so I first thought of making that function do this transposition.
That might be a good thing to do, but it wouldn't be enough to
fix the problem, because we'd then reach this in object_ownercheck:

rel = table_open(classid, AccessShareLock);

which is going to examine the wrong catalog. So AFAICS what
we have to do is put this substitution into object_ownercheck,
adding to the four or five places that know about it already.

This is an absolutely horrid mess, of course. The big problem
is that at this point I have exactly zero confidence that there
are not other places with the same bug; and it's not apparent
how to find them.

There seems little choice but to make the hacky fix in v16,
but I wonder whether we shouldn't be more ambitious and try
to fix this permanently in HEAD, by getting rid of the
discrepancy in which OID to use. ISTM the correct fix
is to change the ObjectAddress representation of large
objects to use classid LargeObjectMetadataRelationId.
Somebody seems to have felt that that would create more
problems than it solves, but I have to disagree. If we
stick with the current way, we are going to be hitting
problems of this ilk forevermore.

Thoughts?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2023-12-13 19:58:51 Re: Windows default locale vs initdb
Previous Message Tom Lane 2023-12-13 18:36:14 Re: Teach predtest about IS [NOT] <boolean> proofs