Re: [PATCH] Largeobject access controls - Mailing list pgsql-hackers
From | KaiGai Kohei |
---|---|
Subject | Re: [PATCH] Largeobject access controls |
Date | |
Msg-id | 4AD672AE.4080208@ak.jp.nec.com Whole thread Raw |
In response to | Re: [PATCH] Largeobject access controls (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
Tom Lane wrote: >> The newer basis is same as other database objects, such as functions. >> But why pg_dump performs correctly for these database objects? > > The reason pg_dump works reasonably well is that it takes locks on > tables, and the other objects it dumps (such as functions) have > indivisible one-row representations in the catalogs. They're either > there when pg_dump looks, or they're not. What you would have here > is that pg_dump would see LO data chunks when it looks into > pg_largeobject using its transaction snapshot, and then its attempts to > open those LO OIDs would fail because the metadata is not there anymore > according to SnapshotNow. It needs to break down the matter more simple. This patch adds the pg_largeobject_metadata catalog, and a certain entry within the catalog is refered by multiple entries within pg_largeobject. At the viewpoint of data model, it is equivalent that any pg_largeobject tuples which share same LOID always have common copy of the metadata. (If we tries to implement this actually, it needs to keep consistency of the part of metadata for each writer operations!) In the current design, when we access a certain large object with read-only mode, query's snapshot is used, not always SnapshotNow. If we assume any data chunk has its metadata part, the metadata should be also refered from the query's snapshot. In fact, this patch stores the part of metadata within pg_largeobject_metadata. But it seems to me the principle is that same snapshot which used for data chunks should be applied to scan the metadata. So, I can agree the following approach: > So I'm kind of inclined to say that the least evil solution is to apply > the permissions check using the query-snapshot-time metadata row. > It's definitely a debatable question though. We'd also want to make sure > that the aclcheck code doesn't fail if it finds a nonexistent role ID > in the ACL. In this approach, we cannot apply the current pg_largeobject_aclcheck() because it does not have an argument to deliver the preferable snapshot. So, we need to extract acl field from the tuple being visible from the appropriate snapshot, then calls aclmask() routine. The aclmask() just compares identifiers in numeris representation, so it seems to me this routine does not raise an error if it finds a nonexistent role ID from the viewpoint of SnapshotNow. aclmask(const Acl *acl, Oid roleid, Oid ownerId, AclMode mask, AclMaskHow how) It requires five arguments. The acl and ownerId can be fetched from the metadata with query's snapshot. The roleid can be given by GetUserId(). The mask and how are constant. An I missing something? > We do not commit system changes that lack necessary pg_dump support. > There are some things you can leave till later, but pg_dump is not > negotiable. (Otherwise, testers would be left with databases they > can't dump/reload across the next forced initdb.) OK, I'll add support pg_dump soon. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>
pgsql-hackers by date: