(2009/12/18 15:48), Takahiro Itagaki wrote:
> 
> Robert Haas<robertmh...@gmail.com>  wrote:
> 
>> In both cases, I'm lost.  Help?
> 
> They might be contrasted with the comments for myLargeObjectExists.
> Since we use MVCC visibility in loread(), metadata for large object
> also should be visible in MVCC rule.
> 
> If I understand them, they say:
>    * pg_largeobject_aclmask_snapshot requires a snapshot which will be
>      used in loread().
>    * Don't use LargeObjectExists if you need MVCC visibility.

Yes, correct.

>> In acldefault(), there is this comment:
>>    /* Grant SELECT,UPDATE by default, for now */
>> This doesn't seem to match what the code is doing, so I think we
>> should remove it.
> 
> Ah, ACL_NO_RIGHTS is the default.

Oops, it reflects very early phase design, but fixed later.

>> I also notice that dumpBlobComments() is now misnamed, but it seems
>> we've chosen to add a comment mentioning that fact rather than fixing it.
> 
> Hmmm, now it dumps not only comments but also ownership of large objects.
> Should we rename it dumpBlobMetadata() or so?

It seems to me quite natural.

The attached patch fixes them.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kai...@ak.jp.nec.com>
*** base/src/backend/utils/adt/acl.c	(revision 2503)
--- base/src/backend/utils/adt/acl.c	(working copy)
***************
*** 765,771 ****
  			owner_default = ACL_ALL_RIGHTS_LANGUAGE;
  			break;
  		case ACL_OBJECT_LARGEOBJECT:
- 			/* Grant SELECT,UPDATE by default, for now */
  			world_default = ACL_NO_RIGHTS;
  			owner_default = ACL_ALL_RIGHTS_LARGEOBJECT;
  			break;
--- 765,770 ----
*** base/src/bin/pg_dump/pg_dump.h	(revision 2503)
--- base/src/bin/pg_dump/pg_dump.h	(working copy)
***************
*** 116,122 ****
  	DO_FOREIGN_SERVER,
  	DO_DEFAULT_ACL,
  	DO_BLOBS,
! 	DO_BLOB_COMMENTS
  } DumpableObjectType;
  
  typedef struct _dumpableObject
--- 116,122 ----
  	DO_FOREIGN_SERVER,
  	DO_DEFAULT_ACL,
  	DO_BLOBS,
! 	DO_BLOB_METADATA
  } DumpableObjectType;
  
  typedef struct _dumpableObject
*** base/src/bin/pg_dump/pg_dump_sort.c	(revision 2503)
--- base/src/bin/pg_dump/pg_dump_sort.c	(working copy)
***************
*** 56,62 ****
  	4,							/* DO_FOREIGN_SERVER */
  	17,							/* DO_DEFAULT_ACL */
  	10,							/* DO_BLOBS */
! 	11							/* DO_BLOB_COMMENTS */
  };
  
  /*
--- 56,62 ----
  	4,							/* DO_FOREIGN_SERVER */
  	17,							/* DO_DEFAULT_ACL */
  	10,							/* DO_BLOBS */
! 	11							/* DO_BLOB_METADATA */
  };
  
  /*
***************
*** 93,99 ****
  	15,							/* DO_FOREIGN_SERVER */
  	27,							/* DO_DEFAULT_ACL */
  	20,							/* DO_BLOBS */
! 	21							/* DO_BLOB_COMMENTS */
  };
  
  
--- 93,99 ----
  	15,							/* DO_FOREIGN_SERVER */
  	27,							/* DO_DEFAULT_ACL */
  	20,							/* DO_BLOBS */
! 	21							/* DO_BLOB_METADATA */
  };
  
  
***************
*** 1151,1159 ****
  					 "BLOBS  (ID %d)",
  					 obj->dumpId);
  			return;
! 		case DO_BLOB_COMMENTS:
  			snprintf(buf, bufsize,
! 					 "BLOB COMMENTS  (ID %d)",
  					 obj->dumpId);
  			return;
  	}
--- 1151,1159 ----
  					 "BLOBS  (ID %d)",
  					 obj->dumpId);
  			return;
! 		case DO_BLOB_METADATA:
  			snprintf(buf, bufsize,
! 					 "BLOB METADATA  (ID %d)",
  					 obj->dumpId);
  			return;
  	}
*** base/src/bin/pg_dump/pg_dump.c	(revision 2503)
--- base/src/bin/pg_dump/pg_dump.c	(working copy)
***************
*** 191,197 ****
  static const char *fmtQualifiedId(const char *schema, const char *id);
  static bool hasBlobs(Archive *AH);
  static int	dumpBlobs(Archive *AH, void *arg);
! static int	dumpBlobComments(Archive *AH, void *arg);
  static void dumpDatabase(Archive *AH);
  static void dumpEncoding(Archive *AH);
  static void dumpStdStrings(Archive *AH);
--- 191,197 ----
  static const char *fmtQualifiedId(const char *schema, const char *id);
  static bool hasBlobs(Archive *AH);
  static int	dumpBlobs(Archive *AH, void *arg);
! static int	dumpBlobMetadata(Archive *AH, void *arg);
  static void dumpDatabase(Archive *AH);
  static void dumpEncoding(Archive *AH);
  static void dumpStdStrings(Archive *AH);
***************
*** 707,716 ****
  		blobobj->name = strdup("BLOBS");
  
  		blobcobj = (DumpableObject *) malloc(sizeof(DumpableObject));
! 		blobcobj->objType = DO_BLOB_COMMENTS;
  		blobcobj->catId = nilCatalogId;
  		AssignDumpId(blobcobj);
! 		blobcobj->name = strdup("BLOB COMMENTS");
  		addObjectDependency(blobcobj, blobobj->dumpId);
  	}
  
--- 707,716 ----
  		blobobj->name = strdup("BLOBS");
  
  		blobcobj = (DumpableObject *) malloc(sizeof(DumpableObject));
! 		blobcobj->objType = DO_BLOB_METADATA;
  		blobcobj->catId = nilCatalogId;
  		AssignDumpId(blobcobj);
! 		blobcobj->name = strdup("BLOB METADATA");
  		addObjectDependency(blobcobj, blobobj->dumpId);
  	}
  
***************
*** 2048,2064 ****
  }
  
  /*
!  * dumpBlobComments
!  *	dump all blob properties.
!  *  It has "BLOB COMMENTS" tag due to the historical reason, but note
!  *  that it is the routine to dump all the properties of blobs.
   *
   * Since we don't provide any way to be selective about dumping blobs,
!  * there's no need to be selective about their comments either.  We put
!  * all the comments into one big TOC entry.
   */
  static int
! dumpBlobComments(Archive *AH, void *arg)
  {
  	const char *blobQry;
  	const char *blobFetchQry;
--- 2048,2062 ----
  }
  
  /*
!  * dumpBlobMetadata
!  *	dump all blob metadata.
   *
   * Since we don't provide any way to be selective about dumping blobs,
!  * there's no need to be selective about their metadata either.  We put
!  * all the metadata into one big TOC entry.
   */
  static int
! dumpBlobMetadata(Archive *AH, void *arg)
  {
  	const char *blobQry;
  	const char *blobFetchQry;
***************
*** 6294,6306 ****
  						 dobj->dependencies, dobj->nDeps,
  						 dumpBlobs, NULL);
  			break;
! 		case DO_BLOB_COMMENTS:
  			ArchiveEntry(fout, dobj->catId, dobj->dumpId,
  						 dobj->name, NULL, NULL, "",
! 						 false, "BLOB COMMENTS", SECTION_DATA,
  						 "", "", NULL,
  						 dobj->dependencies, dobj->nDeps,
! 						 dumpBlobComments, NULL);
  			break;
  	}
  }
--- 6292,6304 ----
  						 dobj->dependencies, dobj->nDeps,
  						 dumpBlobs, NULL);
  			break;
! 		case DO_BLOB_METADATA:
  			ArchiveEntry(fout, dobj->catId, dobj->dumpId,
  						 dobj->name, NULL, NULL, "",
! 						 false, "BLOB METADATA", SECTION_DATA,
  						 "", "", NULL,
  						 dobj->dependencies, dobj->nDeps,
! 						 dumpBlobMetadata, NULL);
  			break;
  	}
  }
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to