On Tue, 27 Sept 2022 at 11:28, David Rowley <dgrowle...@gmail.com> wrote:
> Maybe we could remove the datumCopy() from eval_windowfunction() and
> also document that a window function when returning a non-byval type,
> must allocate the Datum in either ps_ExprContext's
> ecxt_per_tuple_memory or ecxt_per_query_memory. We could ensure any
> extension which has its own window functions get the memo about the
> API change by adding an Assert to ensure that the return value (for
> byref types) is in the current context by calling the
> loop-over-the-blocks version of MemoryContextContains().

I did some work on this and it turned out that the value returned by
any of lead(), lag(), first_value(), last_value() and nth_value()
could also be in MessageContext or some child context to
CacheMemoryContext.  The reason for the latter two is that cases like
LAG(col, 1, 'default value') will return the Const in the 3rd arg when
the offset value is outside of the window frame. That means
MessageContext for normal queries and it means it'll be cached in a
child context of CacheMemoryContext for PREPAREd queries.

This means the Assert that I wanted to add to eval_windowfunction
became quite complex. Namely:

Assert(perfuncstate->resulttypeByVal || fcinfo->isnull ||
   MemoryContextContains(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory,
(void *) *result) ||
   MemoryContextContains(winstate->ss.ps.ps_ExprContext->ecxt_per_query_memory,
(void *) *result) ||
   MemoryContextContains(MessageContext, (void *) *result) ||
   MemoryContextOrChildOfContains(CacheMemoryContext, (void *) *result));

Notice the invention of MemoryContextOrChildOfContains() to
recursively search the CacheMemoryContext children. It does not seem
so great as CacheMemoryContext tends to have many children and
searching through them all could make that Assert a bit slow.

I think I am fairly happy that all the 4 message contexts I mentioned
in the Assert will be around long enough for the result value to not
be freed. It's just that the whole thing feels a bit wrong and that
the context the return value is in should be a bit more predictable.

Does anyone have any opinions on this?

David
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index fe74e49814..bc7eeaf84d 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -1107,6 +1107,11 @@ finalize_aggregate(AggState *aggstate,
                {
                        *resultVal = FunctionCallInvoke(fcinfo);
                        *resultIsNull = fcinfo->isnull;
+
+                       /* ensure by-ref types are allocated in 
CurrentMemoryContext */
+                       Assert(peragg->resulttypeByVal || *resultIsNull ||
+                                  MemoryContextContains(CurrentMemoryContext, 
(void *) *resultVal));
+
                }
                aggstate->curperagg = NULL;
        }
@@ -1115,17 +1120,13 @@ finalize_aggregate(AggState *aggstate,
                /* Don't need MakeExpandedObjectReadOnly; datumCopy will copy 
it */
                *resultVal = pergroupstate->transValue;
                *resultIsNull = pergroupstate->transValueIsNull;
-       }
 
-       /*
-        * If result is pass-by-ref, make sure it is in the right context.
-        */
-       if (!peragg->resulttypeByVal && !*resultIsNull &&
-               !MemoryContextContains(CurrentMemoryContext,
-                                                          
DatumGetPointer(*resultVal)))
-               *resultVal = datumCopy(*resultVal,
-                                                          
peragg->resulttypeByVal,
-                                                          
peragg->resulttypeLen);
+               /* if result is pass-by-ref, copy into the CurrentMemoryContext 
*/
+               if (!peragg->resulttypeByVal && !*resultIsNull)
+                       *resultVal = datumCopy(*resultVal,
+                                                                  
peragg->resulttypeByVal,
+                                                                  
peragg->resulttypeLen);
+       }
 
        MemoryContextSwitchTo(oldContext);
 }
@@ -1172,6 +1173,11 @@ finalize_partialaggregate(AggState *aggstate,
 
                        *resultVal = FunctionCallInvoke(fcinfo);
                        *resultIsNull = fcinfo->isnull;
+
+                       /* ensure by-ref types are allocated in 
CurrentMemoryContext */
+                       Assert(peragg->resulttypeByVal || *resultIsNull ||
+                                  MemoryContextContains(CurrentMemoryContext, 
(void *) *resultVal));
+
                }
        }
        else
@@ -1179,15 +1185,14 @@ finalize_partialaggregate(AggState *aggstate,
                /* Don't need MakeExpandedObjectReadOnly; datumCopy will copy 
it */
                *resultVal = pergroupstate->transValue;
                *resultIsNull = pergroupstate->transValueIsNull;
-       }
 
-       /* If result is pass-by-ref, make sure it is in the right context. */
-       if (!peragg->resulttypeByVal && !*resultIsNull &&
-               !MemoryContextContains(CurrentMemoryContext,
-                                                          
DatumGetPointer(*resultVal)))
-               *resultVal = datumCopy(*resultVal,
-                                                          
peragg->resulttypeByVal,
-                                                          
peragg->resulttypeLen);
+               /* if result is pass-by-ref, copy into the CurrentMemoryContext 
*/
+               if (!peragg->resulttypeByVal && !*resultIsNull)
+                       *resultVal = datumCopy(*resultVal,
+                                                                  
peragg->resulttypeByVal,
+                                                                  
peragg->resulttypeLen);
+
+       }
 
        MemoryContextSwitchTo(oldContext);
 }
diff --git a/src/backend/executor/nodeWindowAgg.c 
b/src/backend/executor/nodeWindowAgg.c
index 8b0858e9f5..b196328060 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -626,6 +626,10 @@ finalize_windowaggregate(WindowAggState *winstate,
                        *result = FunctionCallInvoke(fcinfo);
                        winstate->curaggcontext = NULL;
                        *isnull = fcinfo->isnull;
+
+                       /* ensure by-ref types are allocated in 
CurrentMemoryContext */
+                       Assert(peraggstate->resulttypeByVal || *isnull ||
+                                  MemoryContextContains(CurrentMemoryContext, 
(void *) *result));
                }
        }
        else
@@ -633,17 +637,14 @@ finalize_windowaggregate(WindowAggState *winstate,
                /* Don't need MakeExpandedObjectReadOnly; datumCopy will copy 
it */
                *result = peraggstate->transValue;
                *isnull = peraggstate->transValueIsNull;
+
+               /* if result is pass-by-ref, copy into the CurrentMemoryContext 
*/
+               if (!peraggstate->resulttypeByVal && !*isnull)
+                       *result = datumCopy(*result,
+                                                               
peraggstate->resulttypeByVal,
+                                                               
peraggstate->resulttypeLen);
        }
 
-       /*
-        * If result is pass-by-ref, make sure it is in the right context.
-        */
-       if (!peraggstate->resulttypeByVal && !*isnull &&
-               !MemoryContextContains(CurrentMemoryContext,
-                                                          
DatumGetPointer(*result)))
-               *result = datumCopy(*result,
-                                                       
peraggstate->resulttypeByVal,
-                                                       
peraggstate->resulttypeLen);
        MemoryContextSwitchTo(oldContext);
 }
 
@@ -1034,9 +1035,13 @@ eval_windowfunction(WindowAggState *winstate, 
WindowStatePerFunc perfuncstate,
 {
        LOCAL_FCINFO(fcinfo, FUNC_MAX_ARGS);
        MemoryContext oldContext;
+       WindowResultInfo resultinfo;
 
        oldContext = 
MemoryContextSwitchTo(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory);
 
+       resultinfo.resulttypeLen = perfuncstate->resulttypeLen;
+       resultinfo.resulttypeByVal = perfuncstate->resulttypeByVal;
+
        /*
         * We don't pass any normal arguments to a window function, but we do 
pass
         * it the number of arguments, in order to permit window function
@@ -1046,7 +1051,8 @@ eval_windowfunction(WindowAggState *winstate, 
WindowStatePerFunc perfuncstate,
        InitFunctionCallInfoData(*fcinfo, &(perfuncstate->flinfo),
                                                         
perfuncstate->numArguments,
                                                         
perfuncstate->winCollation,
-                                                        (void *) 
perfuncstate->winobj, NULL);
+                                                        (void *) 
perfuncstate->winobj,
+                                                        (void *) &resultinfo);
        /* Just in case, make all the regular argument slots be null */
        for (int argno = 0; argno < perfuncstate->numArguments; argno++)
                fcinfo->args[argno].isnull = true;
@@ -1057,16 +1063,15 @@ eval_windowfunction(WindowAggState *winstate, 
WindowStatePerFunc perfuncstate,
        *isnull = fcinfo->isnull;
 
        /*
-        * Make sure pass-by-ref data is allocated in the appropriate context. 
(We
-        * need this in case the function returns a pointer into some 
short-lived
-        * tuple, as is entirely possible.)
+        * Ensure byref Datums are allocated in the tuple or query context.  The
+        * result may also be a Const which is stored in the MessageContext or
+        * cached by some cached plan.
         */
-       if (!perfuncstate->resulttypeByVal && !fcinfo->isnull &&
-               !MemoryContextContains(CurrentMemoryContext,
-                                                          
DatumGetPointer(*result)))
-               *result = datumCopy(*result,
-                                                       
perfuncstate->resulttypeByVal,
-                                                       
perfuncstate->resulttypeLen);
+       Assert(perfuncstate->resulttypeByVal || fcinfo->isnull ||
+                  
MemoryContextContains(winstate->ss.ps.ps_ExprContext->ecxt_per_tuple_memory, 
(void *) *result) ||
+                  
MemoryContextContains(winstate->ss.ps.ps_ExprContext->ecxt_per_query_memory, 
(void *) *result) ||
+                  MemoryContextContains(MessageContext, (void *) *result) ||
+                  MemoryContextOrChildOfContains(CacheMemoryContext, (void *) 
*result));
 
        MemoryContextSwitchTo(oldContext);
 }
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 85ee9ec426..f9eaa413e9 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6643,7 +6643,7 @@ int2int4_sum(PG_FUNCTION_ARGS)
        if (transdata->count == 0)
                PG_RETURN_NULL();
 
-       PG_RETURN_DATUM(Int64GetDatumFast(transdata->sum));
+       PG_RETURN_DATUM(Int64GetDatum(transdata->sum));
 }
 
 
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index ec423375ae..a67d72016e 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -1333,6 +1333,28 @@ AllocSetGetChunkContext(void *pointer)
        return &set->header;
 }
 
+#ifdef MEMORY_CONTEXT_CHECKING
+/*
+ * AllocSetContains
+ *             Attempt to determine if 'pointer' is memory which was allocated 
by
+ *             'context'.  Return true if it is, otherwise false.
+ */
+bool
+AllocSetContains(MemoryContext context, void *pointer)
+{
+       AllocSet        set = (AllocSet) context;
+
+       for (AllocBlock blk = set->blocks; blk != NULL; blk = blk->next)
+       {
+               /* see if pointer is in range of this block */
+               if ((char *) pointer >= (char *) blk + ALLOC_BLOCKHDRSZ + 
ALLOC_CHUNKHDRSZ &&
+                       (char *) pointer < blk->endptr)
+                       return true;
+       }
+       return false;
+}
+#endif                                                 /* 
MEMORY_CONTEXT_CHECKING */
+
 /*
  * AllocSetGetChunkSpace
  *             Given a currently-allocated chunk, determine the total space
diff --git a/src/backend/utils/mmgr/generation.c 
b/src/backend/utils/mmgr/generation.c
index c743b24fa7..e44f765f35 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -848,6 +848,32 @@ GenerationGetChunkContext(void *pointer)
        return &block->context->header;
 }
 
+
+#ifdef MEMORY_CONTEXT_CHECKING
+/*
+ * GenerationContains
+ *             Attempt to determine if 'pointer' is memory which was allocated 
by
+ *             'context'.  Return true if it is, otherwise false.
+ */
+bool
+GenerationContains(MemoryContext context, void *pointer)
+{
+       GenerationContext *set = (GenerationContext *) context;
+       dlist_iter      iter;
+
+       dlist_foreach(iter, &set->blocks)
+       {
+               GenerationBlock *blk = dlist_container(GenerationBlock, node, 
iter.cur);
+
+               /* see if pointer is in range of this block */
+               if ((char *) pointer >= (char *) blk + Generation_BLOCKHDRSZ + 
Generation_CHUNKHDRSZ &&
+                       (char *) pointer < blk->endptr)
+                       return true;
+       }
+       return false;
+}
+#endif                                                 /* 
MEMORY_CONTEXT_CHECKING */
+
 /*
  * GenerationGetChunkSpace
  *             Given a currently-allocated chunk, determine the total space
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 115a64cfe4..637cb1a569 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -49,6 +49,7 @@ static const MemoryContextMethods mcxt_methods[] = {
        [MCTX_ASET_ID].stats = AllocSetStats,
 #ifdef MEMORY_CONTEXT_CHECKING
        [MCTX_ASET_ID].check = AllocSetCheck,
+       [MCTX_ASET_ID].contains = AllocSetContains,
 #endif
 
        /* generation.c */
@@ -63,6 +64,7 @@ static const MemoryContextMethods mcxt_methods[] = {
        [MCTX_GENERATION_ID].stats = GenerationStats,
 #ifdef MEMORY_CONTEXT_CHECKING
        [MCTX_GENERATION_ID].check = GenerationCheck,
+       [MCTX_GENERATION_ID].contains = GenerationContains,
 #endif
 
        /* slab.c */
@@ -76,7 +78,8 @@ static const MemoryContextMethods mcxt_methods[] = {
        [MCTX_SLAB_ID].is_empty = SlabIsEmpty,
        [MCTX_SLAB_ID].stats = SlabStats
 #ifdef MEMORY_CONTEXT_CHECKING
-       ,[MCTX_SLAB_ID].check = SlabCheck
+       ,[MCTX_SLAB_ID].check = SlabCheck,
+       [MCTX_SLAB_ID].contains = SlabContains
 #endif
 };
 
@@ -813,48 +816,51 @@ MemoryContextCheck(MemoryContext context)
 }
 #endif
 
+#ifdef MEMORY_CONTEXT_CHECKING
 /*
  * MemoryContextContains
  *             Detect whether an allocated chunk of memory belongs to a given
  *             context or not.
  *
- * Caution: 'pointer' must point to a pointer which was allocated by a
- * MemoryContext.  It's not safe or valid to use this function on arbitrary
- * pointers as obtaining the MemoryContext which 'pointer' belongs to requires
- * possibly several pointer dereferences.
+ * Caution: this test is reliable as long as 'pointer' does point to
+ * a chunk of memory allocated from *some* context.  If 'pointer' points
+ * at memory obtained in some other way, there is a chance of a segmentation
+ * violation due to accessing the chunk header, which look for in the 8 bytes
+ * prior to the pointer.
  */
 bool
 MemoryContextContains(MemoryContext context, void *pointer)
 {
-       /*
-        * Temporarily make this always return false as we don't yet have a 
fully
-        * baked idea on how to make it work correctly with the new MemoryChunk
-        * code.
-        */
-       return false;
+       if (pointer == NULL)
+               return false;
 
-#ifdef NOT_USED
-       MemoryContext ptr_context;
+       return context->methods->contains(context, pointer);
+}
 
-       /*
-        * NB: We must perform run-time checks here which 
GetMemoryChunkContext()
-        * does as assertions before calling GetMemoryChunkContext().
-        *
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       if (pointer == NULL || pointer != (void *) MAXALIGN(pointer))
-               return false;
+/*
+ * MemoryContextOrChildOfContains
+ *             Recursive version of MemoryContextContains which checks if any 
of
+ *             context's child contexts contains the pointer.
+ */
+bool
+MemoryContextOrChildOfContains(MemoryContext context, void *pointer)
+{
+       if (MemoryContextContains(context, pointer))
+               return true;
+       else
+       {
+               MemoryContext child;
 
-       /*
-        * OK, it's probably safe to look at the context.
-        */
-       ptr_context = GetMemoryChunkContext(pointer);
+               for (child = context->firstchild; child != NULL; child = 
child->nextchild)
+               {
+                       if (MemoryContextOrChildOfContains(child, pointer))
+                               return true;
+               }
+       }
 
-       return ptr_context == context;
-#endif
+       return false;
 }
+#endif                                                 /* 
MEMORY_CONTEXT_CHECKING */
 
 /*
  * MemoryContextCreate
@@ -969,6 +975,8 @@ MemoryContextAlloc(MemoryContext context, Size size)
 
        VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1007,6 +1015,8 @@ MemoryContextAllocZero(MemoryContext context, Size size)
 
        MemSetAligned(ret, 0, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1045,6 +1055,8 @@ MemoryContextAllocZeroAligned(MemoryContext context, Size 
size)
 
        MemSetLoop(ret, 0, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1086,6 +1098,8 @@ MemoryContextAllocExtended(MemoryContext context, Size 
size, int flags)
        if ((flags & MCXT_ALLOC_ZERO) != 0)
                MemSetAligned(ret, 0, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1169,6 +1183,8 @@ palloc(Size size)
 
        VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1202,6 +1218,8 @@ palloc0(Size size)
 
        MemSetAligned(ret, 0, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1241,6 +1259,8 @@ palloc_extended(Size size, int flags)
        if ((flags & MCXT_ALLOC_ZERO) != 0)
                MemSetAligned(ret, 0, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1294,6 +1314,8 @@ repalloc(void *pointer, Size size)
 
        VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1329,6 +1351,8 @@ MemoryContextAllocHuge(MemoryContext context, Size size)
 
        VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
@@ -1368,6 +1392,8 @@ repalloc_huge(void *pointer, Size size)
 
        VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
+       Assert(MemoryContextContains(context, ret));
+
        return ret;
 }
 
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 9149aaafcb..f5ad699760 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -567,6 +567,35 @@ SlabGetChunkContext(void *pointer)
        return &slab->header;
 }
 
+#ifdef MEMORY_CONTEXT_CHECKING
+/*
+ * SlabContains
+ *             Attempt to determine if 'pointer' is memory which was allocated 
by
+ *             'context'.  Return true if it is, otherwise false.
+ */
+bool
+SlabContains(MemoryContext context, void *pointer)
+{
+       SlabContext *set = (SlabContext *) context;
+
+       for (int i = 0; i <= set->chunksPerBlock; i++)
+       {
+               dlist_iter      iter;
+
+               dlist_foreach(iter, &set->freelist[i])
+               {
+                       SlabBlock  *blk = dlist_container(SlabBlock, node, 
iter.cur);
+
+                       /* see if pointer is in range of this block */
+                       if ((char *) pointer >= (char *) blk + Slab_BLOCKHDRSZ 
+ Slab_CHUNKHDRSZ &&
+                               (char *) pointer < (char *) blk + 
set->blockSize)
+                               return true;
+               }
+       }
+       return false;
+}
+#endif                                                 /* 
MEMORY_CONTEXT_CHECKING */
+
 /*
  * SlabGetChunkSpace
  *             Given a currently-allocated chunk, determine the total space
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index 63d07358cd..c1d6fd5391 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -72,6 +72,7 @@ typedef struct MemoryContextMethods
                                                  bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
        void            (*check) (MemoryContext context);
+       bool            (*contains) (MemoryContext context, void *pointer);
 #endif
 } MemoryContextMethods;
 
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 52bc41ec53..3bfadbd465 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -95,8 +95,10 @@ extern void 
MemoryContextAllowInCriticalSection(MemoryContext context,
 
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void MemoryContextCheck(MemoryContext context);
-#endif
 extern bool MemoryContextContains(MemoryContext context, void *pointer);
+extern bool MemoryContextOrChildOfContains(MemoryContext context,
+                                                                               
   void *pointer);
+#endif
 
 /* Handy macro for copying and assigning context ID ... but note double eval */
 #define MemoryContextCopyAndSetIdentifier(cxt, id) \
diff --git a/src/include/utils/memutils_internal.h 
b/src/include/utils/memutils_internal.h
index 377cee7a84..b767982896 100644
--- a/src/include/utils/memutils_internal.h
+++ b/src/include/utils/memutils_internal.h
@@ -33,6 +33,7 @@ extern void AllocSetStats(MemoryContext context,
                                                  bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void AllocSetCheck(MemoryContext context);
+extern bool AllocSetContains(MemoryContext context, void *pointer);
 #endif
 
 /* These functions implement the MemoryContext API for Generation context. */
@@ -50,6 +51,7 @@ extern void GenerationStats(MemoryContext context,
                                                        bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void GenerationCheck(MemoryContext context);
+extern bool GenerationContains(MemoryContext context, void *pointer);
 #endif
 
 
@@ -68,6 +70,7 @@ extern void SlabStats(MemoryContext context,
                                          bool print_to_stderr);
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void SlabCheck(MemoryContext context);
+extern bool SlabContains(MemoryContext context, void *pointer);
 #endif
 
 /*
diff --git a/src/include/utils/memutils_memorychunk.h 
b/src/include/utils/memutils_memorychunk.h
index 2eefc138e3..a6a0137e5d 100644
--- a/src/include/utils/memutils_memorychunk.h
+++ b/src/include/utils/memutils_memorychunk.h
@@ -107,9 +107,18 @@
                                                                 
MEMORYCHUNK_VALUE_BASEBIT << \
                                                                 
MEMORYCHUNK_VALUE_BASEBIT)
 
+#ifdef MEMORY_CONTEXT_CHECKING
+#if SIZEOF_SIZE_T >= 8
+#define MEMORYCHUNK_MAGIC2             UINT64CONST(0xB1A8DB858EB6EFBA)
+#else
+#define MEMORYCHUNK_MAGIC2             0x8EB6EFBA
+#endif
+#endif
+
 typedef struct MemoryChunk
 {
 #ifdef MEMORY_CONTEXT_CHECKING
+       Size            chunk_magic;    /* magic number */
        Size            requested_size;
 #endif
 
@@ -161,6 +170,9 @@ MemoryChunkSetHdrMask(MemoryChunk *chunk, void *block,
        Assert(value <= MEMORYCHUNK_MAX_VALUE);
        Assert((int) methodid <= MEMORY_CONTEXT_METHODID_MASK);
 
+#ifdef MEMORY_CONTEXT_CHECKING
+       chunk->chunk_magic = MEMORYCHUNK_MAGIC2;
+#endif
        chunk->hdrmask = (((uint64) blockoffset) << 
MEMORYCHUNK_BLOCKOFFSET_BASEBIT) |
                (((uint64) value) << MEMORYCHUNK_VALUE_BASEBIT) |
                methodid;
@@ -177,6 +189,9 @@ MemoryChunkSetHdrMaskExternal(MemoryChunk *chunk,
 {
        Assert((int) methodid <= MEMORY_CONTEXT_METHODID_MASK);
 
+#ifdef MEMORY_CONTEXT_CHECKING
+       chunk->chunk_magic = MEMORYCHUNK_MAGIC2;
+#endif
        chunk->hdrmask = MEMORYCHUNK_MAGIC | (((uint64) 1) << 
MEMORYCHUNK_EXTERNAL_BASEBIT) |
                methodid;
 }
@@ -194,6 +209,7 @@ MemoryChunkIsExternal(MemoryChunk *chunk)
         */
        Assert(!HdrMaskIsExternal(chunk->hdrmask) ||
                   HdrMaskCheckMagic(chunk->hdrmask));
+       Assert(chunk->chunk_magic == MEMORYCHUNK_MAGIC2);
 
        return HdrMaskIsExternal(chunk->hdrmask);
 }
@@ -207,6 +223,7 @@ static inline Size
 MemoryChunkGetValue(MemoryChunk *chunk)
 {
        Assert(!HdrMaskIsExternal(chunk->hdrmask));
+       Assert(chunk->chunk_magic == MEMORYCHUNK_MAGIC2);
 
        return HdrMaskGetValue(chunk->hdrmask);
 }
@@ -220,6 +237,7 @@ static inline void *
 MemoryChunkGetBlock(MemoryChunk *chunk)
 {
        Assert(!HdrMaskIsExternal(chunk->hdrmask));
+       Assert(chunk->chunk_magic == MEMORYCHUNK_MAGIC2);
 
        return (void *) ((char *) chunk - HdrMaskBlockOffset(chunk->hdrmask));
 }
@@ -229,6 +247,7 @@ MemoryChunkGetBlock(MemoryChunk *chunk)
 #undef MEMORYCHUNK_VALUE_BASEBIT
 #undef MEMORYCHUNK_BLOCKOFFSET_BASEBIT
 #undef MEMORYCHUNK_MAGIC
+#undef MEMORYCHUNK_MAGIC2
 #undef HdrMaskIsExternal
 #undef HdrMaskGetValue
 #undef HdrMaskBlockOffset
diff --git a/src/include/windowapi.h b/src/include/windowapi.h
index 5a620a2e42..c3d65b161c 100644
--- a/src/include/windowapi.h
+++ b/src/include/windowapi.h
@@ -36,7 +36,19 @@
 /* this struct is private in nodeWindowAgg.c */
 typedef struct WindowObjectData *WindowObject;
 
+/*
+ * Evaluation of window functions are passed this object and it's available
+ * via fcinfo->resultinfo and can be accessed in the window function via the
+ * PG_WINDOW_RESULTINFO macro.
+ */
+typedef struct WindowResultInfo
+{
+       int16           resulttypeLen;  /* type length of wfunc's result type */
+       bool            resulttypeByVal;        /* true if wfunc's result is 
byval */
+} WindowResultInfo;
+
 #define PG_WINDOW_OBJECT() ((WindowObject) fcinfo->context)
+#define PG_WINDOW_RESULTINFO() ((WindowResultInfo *) fcinfo->resultinfo)
 
 #define WindowObjectIsValid(winobj) \
        ((winobj) != NULL && IsA(winobj, WindowObjectData))

Reply via email to