
Done - phew

>
> On Mon, 23 Feb 1998, Jan Wieck wrote:
>
> >     STOP
>
>    Damn, a Supreme's song pops into my head *ack*!
>
> >     I already have my fingers on that :-)
> >
> >     GRANT/REVOKE works here now for anything except pg_class. But
> >     I know where it is now. So wait a few minutes. Was a real
> >     submarine bug the system cache (old and ugly since this was
> >     never touched by the regression tests).
>
>    Okay, let me know when fixed and I'll try again :)

    The diff looks so simple and easy. But to find it wasn't fun.

    It must have been there for a long time. What happened:

    When a tuple in one of some central catalogs was updated, the
    referenced  relation  got flushed, so it would be reopened on
    the next access (to reflect new  triggers,  rules  and  table
    structure changes into the relation cache).

    Some  data  (the  tupleDescriptor e.g.) is used in the system
    cache too. So when a relation is subject to the system cache,
    this  must know too that a cached system relation got flushed
    because the tupleDesc data gets freed during the flush!

    For the GRANT/REVOKE on pg_class it was  slightly  different.
    There  is some local data in inval.c that gets initialized on
    the first invalidation of a tuple in some  central  catalogs.
    This  needs a SysCache lookup in pg_class. But when the first
    of all commands is a GRANT on pg_class,  exactly  the  needed
    tuple is the one actually invalidated. So I added little code
    snippets that the initialization of the  local  variables  in
    inval.c will already happen during InitPostgres().


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#======================================== jwieck@debis.com (Jan Wieck) #


diff -c -r /usr/local/pgsql/sup/pgsql/src/include/utils/catcache.h include/utils/catcache.h
*** /usr/local/pgsql/sup/pgsql/src/include/utils/catcache.h	Wed Feb  4 09:34:32 1998
--- include/utils/catcache.h	Mon Feb 23 16:12:36 1998
***************
*** 66,71 ****
--- 66,72 ----
  extern void CatalogCacheIdInvalidate(int cacheId, Index hashIndex,
  						 ItemPointer pointer);
  extern void ResetSystemCache(void);
+ extern void SystemCacheRelationFlushed(Oid relId);
  extern CatCache * InitSysCache(char *relname, char *indname, int id, int nkeys,
  			 int key[], HeapTuple (*iScanfuncP) ());
  extern HeapTuple SearchSysCache(struct catcache * cache, Datum v1, Datum v2,
diff -c -r /usr/local/pgsql/sup/pgsql/src/include/utils/inval.h include/utils/inval.h
*** /usr/local/pgsql/sup/pgsql/src/include/utils/inval.h	Mon Sep  8 23:55:08 1997
--- include/utils/inval.h	Mon Feb 23 16:57:16 1998
***************
*** 16,21 ****
--- 16,23 ----
  #include <access/htup.h>
  #include <utils/rel.h>
  
+ extern void InitLocalInvalidateData(void);
+ 
  extern void DiscardInvalid(void);
  
  extern void RegisterInvalid(bool send);
diff -c -r /usr/local/pgsql/sup/pgsql/src/backend/utils/cache/catcache.c backend/utils/cache/catcache.c
*** /usr/local/pgsql/sup/pgsql/src/backend/utils/cache/catcache.c	Thu Feb 12 14:36:23 1998
--- backend/utils/cache/catcache.c	Mon Feb 23 16:14:59 1998
***************
*** 621,626 ****
--- 621,649 ----
  }
  
  /* --------------------------------
+  *		SystemCacheRelationFlushed
+  *
+  *	RelationFlushRelation() frees some information referenced in the
+  *	cache structures. So we get informed when this is done and arrange
+  *	for the next SearchSysCache() call that this information is setup
+  *	again.
+  * --------------------------------
+  */
+ void
+ SystemCacheRelationFlushed(Oid relId)
+ {
+ 	struct catcache *cache;
+ 
+ 	for (cache = Caches; PointerIsValid(cache); cache = cache->cc_next)
+ 	{
+ 		if (cache->relationId == relId)
+ 		{
+ 			cache->relationId = InvalidOid;
+ 		}
+ 	}
+ }
+ 
+ /* --------------------------------
   *		InitIndexedSysCache
   *
   *	This allocates and initializes a cache for a system catalog relation.
diff -c -r /usr/local/pgsql/sup/pgsql/src/backend/utils/cache/inval.c backend/utils/cache/inval.c
*** /usr/local/pgsql/sup/pgsql/src/backend/utils/cache/inval.c	Tue Nov 18 11:23:07 1997
--- backend/utils/cache/inval.c	Mon Feb 23 16:55:28 1998
***************
*** 512,517 ****
--- 512,531 ----
  	(*function) (relationId, objectId);
  }
  
+ 
+ /*
+  *	InitLocalInvalidateData
+  *
+  *	Setup this before anything could ever get invalid!
+  *	Called by InitPostgres();
+  */
+ void
+ InitLocalInvalidateData()
+ {
+ 	ValidateHacks();
+ }
+ 
+ 
  /*
   * DiscardInvalid --
   *		Causes the invalidated cache state to be discarded.
diff -c -r /usr/local/pgsql/sup/pgsql/src/backend/utils/cache/relcache.c backend/utils/cache/relcache.c
*** /usr/local/pgsql/sup/pgsql/src/backend/utils/cache/relcache.c	Wed Feb  4 09:34:06 1998
--- backend/utils/cache/relcache.c	Mon Feb 23 16:15:20 1998
***************
*** 65,70 ****
--- 65,71 ----
  #include "utils/relcache.h"
  #include "utils/hsearch.h"
  #include "utils/relcache.h"
+ #include "utils/catcache.h"
  
  #include "catalog/catname.h"
  #include "catalog/catalog.h"
***************
*** 1344,1349 ****
--- 1345,1351 ----
  		RelationCacheDelete(relation);
  
  		FreeTupleDesc(relation->rd_att);
+ 		SystemCacheRelationFlushed(relation->rd_id);
  
  		FreeTriggerDesc(relation);
  
diff -c -r /usr/local/pgsql/sup/pgsql/src/backend/utils/init/postinit.c backend/utils/init/postinit.c
*** /usr/local/pgsql/sup/pgsql/src/backend/utils/init/postinit.c	Wed Feb  4 09:34:08 1998
--- backend/utils/init/postinit.c	Mon Feb 23 16:57:05 1998
***************
*** 62,67 ****
--- 62,68 ----
  #include "utils/elog.h"
  #include "utils/palloc.h"
  #include "utils/mcxt.h"			/* for EnableMemoryContext, etc. */
+ #include "utils/inval.h"
  
  #include "catalog/catname.h"
  #include "catalog/pg_database.h"
***************
*** 585,590 ****
--- 586,597 ----
  	 * ----------------
  	 */
  	InitUserid();
+ 
+ 	/* ----------------
+ 	 *	 initialize local data in cache invalidation stuff
+ 	 * ----------------
+ 	 */
+ 	InitLocalInvalidateData();
  
  	/* ----------------
  	 *	ok, all done, now let's make sure we don't do it again.


Mon, 23 Feb 1998 17:19:14 +0100 (MET)
orion.SAPserv.Hamburg.dsh.de!wieck@sapserv.debis.de
Re: [HACKERS] Here it is - view permissions