Largeobject Access Controls (r2460)
in [PgSql]
|
Prev: [HACKERS] is any reason why we cannot cast from record (row) to typed row?
Next: [HACKERS] 8.5 vs. 9.0
From: KaiGai Kohei on 1 Feb 2010 20:55 (2010/02/02 9:33), Takahiro Itagaki wrote: > > KaiGai Kohei<kaigai(a)ak.jp.nec.com> wrote: > >>> Can we remove such path and raise an error instead? >>> Also, even if we support the older servers in the routine, >>> the new bytea format will be another problem anyway. >> >> OK, I'll fix it. > > I think we might need to discuss about explicit version checks in pg_restore. > It is not related with large objects, but with pg_restore's capability. > We've not supported to restore a dump to older servers, but we don't have any > version checks, right? Should we do the checks at the beginning of restoring? > If we do so, LO patch could be more simplified. I agree it is a good idea. >> The --schema-only with large objects might be unnatural, but the >> --data-only with properties of large objects are also unnatural. >> Which behavior is more unnatural? > > I think large object metadata is a kind of row-based access controls. > How do we dump and restore ACLs per rows when we support them for > normal tables? We should treat LO metadata as same as row-based ACLs. > In my opinion, I'd like to treat them as part of data (not of schema). OK, I'll update the patch according to the behavior you suggested. | I'd prefer to keep the existing behavior: | * default or data-only : dump all attributes and data of blobs | * schema-only : don't dump any blobs | and have independent options to control blob dumps: | * -b, --blobs : dump all blobs even if schema-only | * -B, --no-blobs : [NEW] don't dump any blobs even if default or data-only Please wait for a while. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai(a)ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: KaiGai Kohei on 1 Feb 2010 23:49 >>> The --schema-only with large objects might be unnatural, but the >>> --data-only with properties of large objects are also unnatural. >>> Which behavior is more unnatural? >> >> I think large object metadata is a kind of row-based access controls. >> How do we dump and restore ACLs per rows when we support them for >> normal tables? We should treat LO metadata as same as row-based ACLs. >> In my opinion, I'd like to treat them as part of data (not of schema). > > OK, I'll update the patch according to the behavior you suggested. > | I'd prefer to keep the existing behavior: > | * default or data-only : dump all attributes and data of blobs > | * schema-only : don't dump any blobs > | and have independent options to control blob dumps: > | * -b, --blobs : dump all blobs even if schema-only > | * -B, --no-blobs : [NEW] don't dump any blobs even if default or data-only I found out it needs special treatments to dump comments and access privileges of blobs. See dumpACL() and dumpComment() | static void | dumpACL(Archive *fout, CatalogId objCatId, DumpId objDumpId, | const char *type, const char *name, const char *subname, | const char *tag, const char *nspname, const char *owner, | const char *acls) | { | PQExpBuffer sql; | | /* Do nothing if ACL dump is not enabled */ | if (dataOnly || aclsSkip) | return; | : | static void | dumpComment(Archive *fout, const char *target, | const char *namespace, const char *owner, | CatalogId catalogId, int subid, DumpId dumpId) | { | CommentItem *comments; | int ncomments; | | /* Comments are SCHEMA not data */ | if (dataOnly) | return; | : In addition, _printTocEntry() is not called with acl_pass = true, when --data-only is given. I again wonder whether we are on the right direction. Originally, the reason why we decide to use per blob toc entry was that "BLOB ACLS" entry needs a few exceptional treatments in the code. But, if we deal with "BLOB ITEM" entry as data contents, it will also need additional exceptional treatments. Indeed, even if we have row-level ACLs, it will be dumped in data section without separating them into properties and data contents because of the restriction on implementation, not a data modeling reason. Many of empty large objects may not be sane situation, but it is suitable for the existing manner in pg_dump/pg_restore, at least. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai(a)ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 3 Feb 2010 10:20 2010/2/1 KaiGai Kohei <kaigai(a)ak.jp.nec.com>: > I again wonder whether we are on the right direction. I believe the proposed approach is to dump blob metadata if and only if you are also dumping blob contents, and to do all of this for data dumps but not schema dumps. That seems about right to me. > Originally, the reason why we decide to use per blob toc entry was > that "BLOB ACLS" entry needs a few exceptional treatments in the code. > But, if we deal with "BLOB ITEM" entry as data contents, it will also > need additional exceptional treatments. But the new ones are less objectionable, maybe. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: KaiGai Kohei on 4 Feb 2010 03:30 (2010/02/04 0:20), Robert Haas wrote: > 2010/2/1 KaiGai Kohei<kaigai(a)ak.jp.nec.com>: >> I again wonder whether we are on the right direction. > > I believe the proposed approach is to dump blob metadata if and only > if you are also dumping blob contents, and to do all of this for data > dumps but not schema dumps. That seems about right to me. In other words: <default> -> blob contents and metadata (owner, acl, comments) shall be dumped --data-only -> only blob contents shall be dumped --schema-only -> neither blob contents and metadata are dumped. Can I understand correctly? >> Originally, the reason why we decide to use per blob toc entry was >> that "BLOB ACLS" entry needs a few exceptional treatments in the code. >> But, if we deal with "BLOB ITEM" entry as data contents, it will also >> need additional exceptional treatments. > > But the new ones are less objectionable, maybe. > > ...Robert > -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai(a)ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: KaiGai Kohei on 4 Feb 2010 04:38
(2010/02/04 17:30), KaiGai Kohei wrote: > (2010/02/04 0:20), Robert Haas wrote: >> 2010/2/1 KaiGai Kohei<kaigai(a)ak.jp.nec.com>: >>> I again wonder whether we are on the right direction. >> >> I believe the proposed approach is to dump blob metadata if and only >> if you are also dumping blob contents, and to do all of this for data >> dumps but not schema dumps. That seems about right to me. > > In other words: > > <default> -> blob contents and metadata (owner, acl, comments) shall > be dumped > --data-only -> only blob contents shall be dumped > --schema-only -> neither blob contents and metadata are dumped. > > Can I understand correctly? The attached patch enables not to dump "BLOB ITEM" section and corresponding metadata when --data-only is specified. In addition, it does not output both "BLOB DATA" and "BLOB ITEM" section when --schema-only is specified. When --data-only is given to pg_dump, it does not construct any DO_BLOB_ITEM entries in getBlobs(), so all the metadata (owner, acls, comment) are not dumped. And it writes legacy "BLOBS" section instead of the new "BLOB DATA" section to inform pg_restore this archive does not create large objects in "BLOB ITEM" section. If --schema-only is given, getBlobs() is simply skipped. When --data-only is given to pg_restore, it skips all the "BLOB ITEM" sections. Large objects are created in _LoadBlobs() instead of the section, like as we have done until now. The _LoadBlobs() takes the third argument which specifies whether we should create large object here, or not. Its condition is a bit modified from the previous patch. if (strcmp(te->desc, "BLOBS") == 0 || ropt->dataOnly) _LoadBlobs(AH, ropt, true); ^^^^^^^^^^^^^^^^^ else if (strcmp(te->desc, "BLOB DATA") == 0) _LoadBlobs(AH, ropt, false); When --data-only is given to pg_restore, "BLOB ITEM" secition is skipped, so we need to create large objects at _LoadBlobs() stage, even if the archive has "BLOB DATA" section. In addition, --schema-only kills all the "BLOB ITEM" section using a special condition that was added to _tocEntryRequired(). It might be a bit different from what Itagaki-san suggested, because "BLOB ITEM" section is still in SECTION_PRE_DATA section. However, it minimizes special treatments in the code, and no differences from the viewpoint of end-users. Or, is it necessary to pack them into SECTION_DATA section anyway? Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai(a)ak.jp.nec.com> |