RE: [HACKERS] copyObject() ? again

Hello all,

>
> As to the 2nd case I reported,it was probably introduced by my
> patch. The patch was made to solve other Aggregate bugs.
> get_agg_tlist_references() is used to reconstruct aggs member
> node of Agg type nodes as SS_pull_subPlan() does in CopyPlan
> Fields(). (The name of function was set_agg_tlist_references()
> originally. Probably Bruce changed it).
> We may be able to patch by following related code closely.
> But it seems strange that copying objects requires such
> a complicated procedure.
>

I made a patch for the 2nd case I reported.
I applied my trial implementation of copyObject() partially to
the relation between Agg and Aggref type nodes. This patch
solves both this bug and an old bug solved by my old patch
[Subject:SPI_prepare() doesn't work well ? ].

Thanks in advance.

Hiroshi Inoue
Inoue@tpf.co.jp

*** backend/nodes/copyfuncs.c.orig    Tue Feb 23 17:00:26 1999
--- backend/nodes/copyfuncs.c    Thu Mar  4 16:32:21 1999
***************
*** 31,37 ****
--- 31,78 ---- #include "storage/lmgr.h" #include "optimizer/planmain.h"

+ #include "access/xact.h"
+ /*
+  *    Local structures for maintaining the list of
+  *        (old -> new) pairs
+  *        during copyObject operations
+  */
+
+ typedef
+ struct
+ {
+     const void    *old;
+     void        *new;
+ } old_new_pair;
+
+ typedef
+ struct
+ {
+     int    maxcnt;
+     int    count;
+     old_new_pair old_new[1];
+ } old_new_Hash;
+
+ typedef
+ struct
+ {
+     bool        inuse;
+     TransactionId    execXID;
+     unsigned long    alloconce;
+     List        *first;
+     List        *last;
+ } copyContext;
+
+ static    copyContext common_context = {false, 0, 0, NIL, NIL};
+
+ static bool appendToCopyContext(const void *, void *, copyContext *);
+ static void initializeCopyContext(copyContext *, int);
+ static void releaseCopyContext(copyContext *);
+ static void *alreadyCopiedTo(const void *, const copyContext *);
+ static bool isContextStillAlive(const copyContext *);
+
+ /*  * listCopy  *      this copy function only copies the "lcons-cells" of the list but not  *      its contents.
(goodfor list of pointers as well as list of
 
integers).
***************
*** 472,483 **** static Agg * _copyAgg(Agg *from) {     Agg           *newnode = makeNode(Agg);
     CopyPlanFields((Plan *) from, (Plan *) newnode);

!     newnode->aggs = get_agg_tlist_references(newnode);
     return newnode; }

--- 513,535 ---- static Agg * _copyAgg(Agg *from) {
+     bool        init_here = false;
+     copyContext    *copycontext = &common_context;     Agg           *newnode = makeNode(Agg);

+     if (!isContextStillAlive(copycontext))
+     {
+         initializeCopyContext(copycontext,0);
+         init_here = true;
+     }
+     CopyPlanFields((Plan *) from, (Plan *) newnode);

!     Node_Copy(from, newnode, aggs);

+     if (init_here)
+         releaseCopyContext(copycontext);
+     return newnode; }

***************
*** 871,878 **** static Aggref * _copyAggref(Aggref *from) {
!     Aggref       *newnode = makeNode(Aggref);
     /* ----------------      *    copy remainder of node      * ----------------
--- 923,941 ---- static Aggref * _copyAggref(Aggref *from) {
!     copyContext    *copycontext = &common_context;
!     Aggref           *newnode;

+     if (copycontext->inuse)
+     {
+         newnode = (Aggref *)alreadyCopiedTo(from, copycontext);
+         if (newnode)    return newnode;
+     }
+
+     newnode = makeNode(Aggref);
+     if (copycontext->inuse)
+         appendToCopyContext(from, newnode, copycontext);
+     /* ----------------      *    copy remainder of node      * ----------------
***************
*** 1868,1871 ****
--- 1931,2032 ----             break;     }     return retval;
+ }
+
+ /*
+  *    static functions for maintaining the list of
+  *        (old -> new) pairs
+  *        during copyObject operations
+  */
+ #define    _Count_Per_Elem_    127
+ static void initializeCopyContext(copyContext *copycontext, int alloconce)
+ {
+     copycontext->inuse   = true;
+     copycontext->execXID = GetCurrentTransactionId();
+     if (alloconce>0)
+         copycontext->alloconce = alloconce;
+     else
+         copycontext->alloconce = _Count_Per_Elem_;
+     copycontext->first = copycontext->last = NIL;
+ }
+
+ static void releaseCopyContext(copyContext *copycontext)
+ {
+     List        *l;
+
+     if (!copycontext)    return;
+     foreach(l, copycontext->first)
+         pfree(lfirst(l));
+     freeList(copycontext->first);
+     copycontext->inuse = false;
+     copycontext->alloconce = 0;
+     StoreInvalidTransactionId(©context->execXID);
+     copycontext->first = copycontext->last = NIL;
+ }
+
+ static bool appendToCopyContext(const void *old, void *new, copyContext
*copycontext)
+ {
+     old_new_Hash    *copyHash;
+     List        *newl;
+     bool        newList = false;
+
+     if (!copycontext)        return false;
+     if ((!old) || (!new))        return false;
+     if (!copycontext->last)
+         newList = true;
+     else
+     {
+         copyHash = (old_new_Hash *)lfirst(copycontext->last);
+         if (copyHash->count >= copyHash->maxcnt)
+             newList = true;
+     }
+     if (newList)
+     {
+         copyHash = (old_new_Hash *) palloc(
+             sizeof(old_new_Hash) + sizeof(old_new_pair) *
+                 (copycontext->alloconce - 1) );
+         if (!copyHash)        return false;
+         copyHash->maxcnt = copycontext->alloconce;
+         copyHash->count  = 0;
+         if (copycontext->last)
+         {
+             newl = lcons(copyHash, NIL);
+             lnext(copycontext->last) = newl;
+             copycontext->last = newl;
+         }
+         else
+         {
+             newl = lcons(copyHash, NIL);
+             copycontext->first = copycontext->last = newl;
+         }
+     }
+
+     copyHash->old_new[copyHash->count].old = old;
+     copyHash->old_new[copyHash->count].new = new;
+     copyHash->count++;
+     return true;
+ }
+ static void *alreadyCopiedTo(const void *obj, const copyContext
*copycontext)
+ {
+     List            *l;
+     int            i;
+     const old_new_Hash    *copyHash;
+
+     if (!copycontext)    return NULL;
+     foreach(l, copycontext->first)
+     {
+         copyHash = (const old_new_Hash *)lfirst(l);
+         for (i=0; i<copyHash->count; i++)
+         {
+             if ( copyHash->old_new[i].old == obj )
+                 return copyHash->old_new[i].new;
+         }
+     }
+     return NULL;
+ }
+
+ static bool isContextStillAlive(const copyContext *copycontext)
+ {
+     return ( copycontext->inuse &&
+     TransactionIdIsCurrentTransactionId(copycontext->execXID) ) ; }