retire MemoryContextResetAndDeleteChildren backwards compatibility macro
I just found myself researching the difference between MemoryContextReset()
and MemoryContextResetAndDeleteChildren(), and it turns out that as of
commit eaa5808 (2015), there is none.
MemoryContextResetAndDeleteChildren() is just a backwards compatibility
macro for MemoryContextReset(). I found this surprising because it sounds
like they do very different things.
Shall we retire this backwards compatibility macro at this point? A search
of https://codesearch.debian.net/ does reveal a few external uses, so we
could alternatively leave it around and just update Postgres to stop using
it, but I don't think it would be too burdensome for extension authors to
fix if we removed it completely.
Patch attached.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
retire_compatibility_macro_v1.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 25338a90e2..f0eac078e0 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -297,7 +297,7 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
samepage))
{
/* no luck; start over */
- MemoryContextResetAndDeleteChildren(tupcxt);
+ MemoryContextReset(tupcxt);
continue;
}
}
@@ -533,7 +533,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
CHECK_FOR_INTERRUPTS();
- MemoryContextResetAndDeleteChildren(perRangeCxt);
+ MemoryContextReset(perRangeCxt);
tup = brinGetTupleForHeapBlock(opaque->bo_rmAccess, heapBlk, &buf,
&off, &size, BUFFER_LOCK_SHARE);
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index ae7b0e9bb8..f719577695 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -251,7 +251,7 @@ ginFreeScanKeys(GinScanOpaque so)
tbm_free(entry->matchBitmap);
}
- MemoryContextResetAndDeleteChildren(so->keyCtx);
+ MemoryContextReset(so->keyCtx);
so->keys = NULL;
so->nkeys = 0;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 74ce5f9491..7f0c5bf43d 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1933,7 +1933,7 @@ AtCleanup_Memory(void)
* Clear the special abort context for next time.
*/
if (TransactionAbortContext != NULL)
- MemoryContextResetAndDeleteChildren(TransactionAbortContext);
+ MemoryContextReset(TransactionAbortContext);
/*
* Release all transaction-local memory.
@@ -1969,7 +1969,7 @@ AtSubCleanup_Memory(void)
* Clear the special abort context for next time.
*/
if (TransactionAbortContext != NULL)
- MemoryContextResetAndDeleteChildren(TransactionAbortContext);
+ MemoryContextReset(TransactionAbortContext);
/*
* Delete the subxact local memory contexts. Its CurTransactionContext can
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 206d1689ef..e264ffdcf2 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -583,7 +583,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
stats->stadistinct = n_distinct;
}
- MemoryContextResetAndDeleteChildren(col_context);
+ MemoryContextReset(col_context);
}
if (nindexes > 0)
@@ -972,7 +972,7 @@ compute_index_stats(Relation onerel, double totalrows,
numindexrows,
totalindexrows);
- MemoryContextResetAndDeleteChildren(col_context);
+ MemoryContextReset(col_context);
}
}
@@ -981,7 +981,7 @@ compute_index_stats(Relation onerel, double totalrows,
ExecDropSingleTupleTableSlot(slot);
FreeExecutorState(estate);
- MemoryContextResetAndDeleteChildren(ind_context);
+ MemoryContextReset(ind_context);
}
MemoryContextSwitchTo(old_context);
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index e781003934..3207643156 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -317,7 +317,7 @@ ExecReScanRecursiveUnion(RecursiveUnionState *node)
/* Release any hashtable storage */
if (node->tableContext)
- MemoryContextResetAndDeleteChildren(node->tableContext);
+ MemoryContextReset(node->tableContext);
/* Empty hashtable if needed */
if (plan->numCols > 0)
diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c
index 98c1b84d43..5a84969cf8 100644
--- a/src/backend/executor/nodeSetOp.c
+++ b/src/backend/executor/nodeSetOp.c
@@ -631,7 +631,7 @@ ExecReScanSetOp(SetOpState *node)
/* Release any hashtable storage */
if (node->tableContext)
- MemoryContextResetAndDeleteChildren(node->tableContext);
+ MemoryContextReset(node->tableContext);
/* And rebuild empty hashtable if needed */
if (((SetOp *) node->ps.plan)->strategy == SETOP_HASHED)
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 77724a6daa..3258305f57 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -216,7 +216,7 @@ initialize_windowaggregate(WindowAggState *winstate,
* it, so we must leave it to the caller to reset at an appropriate time.
*/
if (peraggstate->aggcontext != winstate->aggcontext)
- MemoryContextResetAndDeleteChildren(peraggstate->aggcontext);
+ MemoryContextReset(peraggstate->aggcontext);
if (peraggstate->initValueIsNull)
peraggstate->transValue = peraggstate->initValue;
@@ -875,7 +875,7 @@ eval_windowaggregates(WindowAggState *winstate)
* result for it, else we'll leak memory.
*/
if (numaggs_restart > 0)
- MemoryContextResetAndDeleteChildren(winstate->aggcontext);
+ MemoryContextReset(winstate->aggcontext);
for (i = 0; i < numaggs; i++)
{
peraggstate = &winstate->peragg[i];
@@ -1351,12 +1351,12 @@ release_partition(WindowAggState *winstate)
* any aggregate temp data). We don't rely on retail pfree because some
* aggregates might have allocated data we don't have direct pointers to.
*/
- MemoryContextResetAndDeleteChildren(winstate->partcontext);
- MemoryContextResetAndDeleteChildren(winstate->aggcontext);
+ MemoryContextReset(winstate->partcontext);
+ MemoryContextReset(winstate->aggcontext);
for (i = 0; i < winstate->numaggs; i++)
{
if (winstate->peragg[i].aggcontext != winstate->aggcontext)
- MemoryContextResetAndDeleteChildren(winstate->peragg[i].aggcontext);
+ MemoryContextReset(winstate->peragg[i].aggcontext);
}
if (winstate->buffer)
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 33975687b3..0e46c59d25 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -547,7 +547,7 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
if (_SPI_current->execSubid >= mySubid)
{
_SPI_current->execSubid = InvalidSubTransactionId;
- MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
+ MemoryContextReset(_SPI_current->execCxt);
}
/* throw away any tuple tables created within current subxact */
@@ -3083,7 +3083,7 @@ _SPI_end_call(bool use_exec)
/* mark Executor context no longer in use */
_SPI_current->execSubid = InvalidSubTransactionId;
/* and free Executor memory */
- MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
+ MemoryContextReset(_SPI_current->execCxt);
}
return 0;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 3a6f24a023..86a3b3d8be 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -555,7 +555,7 @@ AutoVacLauncherMain(int argc, char *argv[])
FlushErrorState();
/* Flush any leaked data in the top-level context */
- MemoryContextResetAndDeleteChildren(AutovacMemCxt);
+ MemoryContextReset(AutovacMemCxt);
/* don't leave dangling pointers to freed memory */
DatabaseListCxt = NULL;
@@ -2521,7 +2521,7 @@ do_autovacuum(void)
/* clean up memory before each iteration */
- MemoryContextResetAndDeleteChildren(PortalContext);
+ MemoryContextReset(PortalContext);
/*
* Save the relation name for a possible error message, to avoid a
@@ -2576,7 +2576,7 @@ do_autovacuum(void)
/* this resets ProcGlobal->statusFlags[i] too */
AbortOutOfAnyTransaction();
FlushErrorState();
- MemoryContextResetAndDeleteChildren(PortalContext);
+ MemoryContextReset(PortalContext);
/* restart our transaction for the following operations */
StartTransactionCommand();
@@ -2718,7 +2718,7 @@ perform_work_item(AutoVacuumWorkItem *workitem)
autovac_report_workitem(workitem, cur_nspname, cur_relname);
/* clean up memory before each work item */
- MemoryContextResetAndDeleteChildren(PortalContext);
+ MemoryContextReset(PortalContext);
/*
* We will abort the current work item if something errors out, and
@@ -2770,7 +2770,7 @@ perform_work_item(AutoVacuumWorkItem *workitem)
/* this resets ProcGlobal->statusFlags[i] too */
AbortOutOfAnyTransaction();
FlushErrorState();
- MemoryContextResetAndDeleteChildren(PortalContext);
+ MemoryContextReset(PortalContext);
/* restart our transaction for the following operations */
StartTransactionCommand();
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index f2e4f23d9f..d02dc17b9c 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -182,7 +182,7 @@ BackgroundWriterMain(void)
FlushErrorState();
/* Flush any leaked data in the top-level context */
- MemoryContextResetAndDeleteChildren(bgwriter_context);
+ MemoryContextReset(bgwriter_context);
/* re-initialize to avoid repeated errors causing problems */
WritebackContextInit(&wb_context, &bgwriter_flush_after);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index a3c1aba24e..42c807d352 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -290,7 +290,7 @@ CheckpointerMain(void)
FlushErrorState();
/* Flush any leaked data in the top-level context */
- MemoryContextResetAndDeleteChildren(checkpointer_context);
+ MemoryContextReset(checkpointer_context);
/* Now we can allow interrupts again */
RESUME_INTERRUPTS();
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 266fbc2339..48bc92205b 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -178,7 +178,7 @@ WalWriterMain(void)
FlushErrorState();
/* Flush any leaked data in the top-level context */
- MemoryContextResetAndDeleteChildren(walwriter_context);
+ MemoryContextReset(walwriter_context);
/* Now we can allow interrupts again */
RESUME_INTERRUPTS();
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 52a9f136ab..21abf34ef7 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3647,7 +3647,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
}
/* Cleanup the memory. */
- MemoryContextResetAndDeleteChildren(ApplyMessageContext);
+ MemoryContextReset(ApplyMessageContext);
MemoryContextSwitchTo(TopMemoryContext);
/* Check if we need to exit the streaming loop. */
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 9f67a57724..7f014a0cbb 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -2237,7 +2237,7 @@ compute_expr_stats(Relation onerel, double totalrows,
ExecDropSingleTupleTableSlot(slot);
FreeExecutorState(estate);
- MemoryContextResetAndDeleteChildren(expr_context);
+ MemoryContextReset(expr_context);
}
MemoryContextSwitchTo(old_context);
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 6a070b5d8c..e415cf1f34 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4457,7 +4457,7 @@ PostgresMain(const char *dbname, const char *username)
* query input buffer in the cleared MessageContext.
*/
MemoryContextSwitchTo(MessageContext);
- MemoryContextResetAndDeleteChildren(MessageContext);
+ MemoryContextReset(MessageContext);
initStringInfo(&input_message);
diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c
index 5a721a4046..879c6d87c3 100644
--- a/src/backend/utils/cache/evtcache.c
+++ b/src/backend/utils/cache/evtcache.c
@@ -91,7 +91,7 @@ BuildEventTriggerCache(void)
* This can happen either because a previous rebuild failed, or
* because an invalidation happened before the rebuild was complete.
*/
- MemoryContextResetAndDeleteChildren(EventTriggerCacheContext);
+ MemoryContextReset(EventTriggerCacheContext);
}
else
{
@@ -262,7 +262,7 @@ InvalidateEventCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
*/
if (EventTriggerCacheState == ETCS_VALID)
{
- MemoryContextResetAndDeleteChildren(EventTriggerCacheContext);
+ MemoryContextReset(EventTriggerCacheContext);
EventTriggerCache = NULL;
}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index eeb238331e..6aeb855e49 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1833,7 +1833,7 @@ FlushErrorState(void)
errordata_stack_depth = -1;
recursion_depth = 0;
/* Delete all data in ErrorContext */
- MemoryContextResetAndDeleteChildren(ErrorContext);
+ MemoryContextReset(ErrorContext);
}
/*
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 21640d62a6..d14e8546a6 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -66,9 +66,6 @@ extern PGDLLIMPORT MemoryContext CurTransactionContext;
/* This is a transient link to the active portal's memory context: */
extern PGDLLIMPORT MemoryContext PortalContext;
-/* Backwards compatibility macro */
-#define MemoryContextResetAndDeleteChildren(ctx) MemoryContextReset(ctx)
-
/*
* Memory-context-type-independent functions in mcxt.c
On Tue, Nov 14, 2023 at 12:30 AM Nathan Bossart <nathandbossart@gmail.com>
wrote:
I just found myself researching the difference between MemoryContextReset()
and MemoryContextResetAndDeleteChildren(), and it turns out that as of
commit eaa5808 (2015), there is none.
MemoryContextResetAndDeleteChildren() is just a backwards compatibility
macro for MemoryContextReset(). I found this surprising because it sounds
like they do very different things.Shall we retire this backwards compatibility macro at this point? A search
of https://codesearch.debian.net/ does reveal a few external uses, so we
could alternatively leave it around and just update Postgres to stop using
it, but I don't think it would be too burdensome for extension authors to
fix if we removed it completely.
+1
Patch attached.
Changes looks pretty much straight forward, but patch failed to apply on the
latest master head(b41b1a7f490) at me.
Regards,
Amul
On Tue, Nov 14, 2023 at 04:25:24PM +0530, Amul Sul wrote:
Changes looks pretty much straight forward, but patch failed to apply on the
latest master head(b41b1a7f490) at me.
Thanks for taking a look. Would you mind sharing the error(s) you are
seeing? The patch applies fine on cfbot and my machine, and check-world
continues to pass.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes:
On Tue, Nov 14, 2023 at 04:25:24PM +0530, Amul Sul wrote:
Changes looks pretty much straight forward, but patch failed to apply on the
latest master head(b41b1a7f490) at me.
Thanks for taking a look. Would you mind sharing the error(s) you are
seeing? The patch applies fine on cfbot and my machine, and check-world
continues to pass.
It may be a question of the tool used to apply the patch. IME,
"patch" is pretty forgiving, "git am" very much less so.
regards, tom lane
On Tue, Nov 14, 2023 at 10:59:04AM -0500, Tom Lane wrote:
Nathan Bossart <nathandbossart@gmail.com> writes:
On Tue, Nov 14, 2023 at 04:25:24PM +0530, Amul Sul wrote:
Changes looks pretty much straight forward, but patch failed to apply on the
latest master head(b41b1a7f490) at me.Thanks for taking a look. Would you mind sharing the error(s) you are
seeing? The patch applies fine on cfbot and my machine, and check-world
continues to pass.It may be a question of the tool used to apply the patch. IME,
"patch" is pretty forgiving, "git am" very much less so.
Ah. I just did a 'git diff > file_name' for this one, so you'd indeed need
to use git-apply instead of git-am. (I ordinarily use git-format-patch,
but I sometimes use git-diff for trivial or prototype patches.)
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On 2023-Nov-13, Nathan Bossart wrote:
Shall we retire this backwards compatibility macro at this point? A search
of https://codesearch.debian.net/ does reveal a few external uses, so we
could alternatively leave it around and just update Postgres to stop using
it, but I don't think it would be too burdensome for extension authors to
fix if we removed it completely.
Let's leave the macro around and just remove its uses in PGDG-owned
code. Having the macro around hurts nothing, and we can remove it in 15
years or so.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Entristecido, Wutra (canción de Las Barreras)
echa a Freyr a rodar
y a nosotros al mar"
On Tue, Nov 14, 2023 at 05:20:16PM +0100, Alvaro Herrera wrote:
Let's leave the macro around and just remove its uses in PGDG-owned
code. Having the macro around hurts nothing, and we can remove it in 15
years or so.
WFM
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
On 2023-Nov-13, Nathan Bossart wrote:
Shall we retire this backwards compatibility macro at this point? A search
of https://codesearch.debian.net/ does reveal a few external uses, so we
could alternatively leave it around and just update Postgres to stop using
it, but I don't think it would be too burdensome for extension authors to
fix if we removed it completely.Let's leave the macro around and just remove its uses in PGDG-owned
code. Having the macro around hurts nothing, and we can remove it in 15
years or so.
Is there a preprocessor symbol that is defined when building Postgres
itself (and extensions in /contrib/), but not third-party extensions (or
vice versa)? If so, the macro could be guarded by that, so that uses
don't accientally sneak back in.
There's also __attribute__((deprecated)) (and and __declspec(deprecated)
for MSVC), but that can AFAIK only be attached to functions and
variables, not macros, so it would have to be changed to a static inline
function.
- ilmari
On Tue, Nov 14, 2023 at 04:36:44PM +0000, Dagfinn Ilmari Manns�ker wrote:
Is there a preprocessor symbol that is defined when building Postgres
itself (and extensions in /contrib/), but not third-party extensions (or
vice versa)? If so, the macro could be guarded by that, so that uses
don't accientally sneak back in.
I'm not aware of anything like that.
There's also __attribute__((deprecated)) (and and __declspec(deprecated)
for MSVC), but that can AFAIK only be attached to functions and
variables, not macros, so it would have to be changed to a static inline
function.
It might be worth introducing pg_attribute_deprecated() in c.h. I'm not
too worried about this particular macro, but it seems handy in general.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, Nov 14, 2023 at 11:01:15AM -0600, Nathan Bossart wrote:
On Tue, Nov 14, 2023 at 04:36:44PM +0000, Dagfinn Ilmari Manns�ker wrote:
There's also __attribute__((deprecated)) (and and __declspec(deprecated)
for MSVC), but that can AFAIK only be attached to functions and
variables, not macros, so it would have to be changed to a static inline
function.It might be worth introducing pg_attribute_deprecated() in c.h. I'm not
too worried about this particular macro, but it seems handy in general.
Huh, this was brought up before [0]/messages/by-id/20200825183002.fkvzxtneijsdgrfv@alap3.anarazel.de.
[0]: /messages/by-id/20200825183002.fkvzxtneijsdgrfv@alap3.anarazel.de
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes:
It might be worth introducing pg_attribute_deprecated() in c.h. I'm not
too worried about this particular macro, but it seems handy in general.
Huh, this was brought up before [0].
[0] /messages/by-id/20200825183002.fkvzxtneijsdgrfv@alap3.anarazel.de
FWIW, I think it's fine to just nuke MemoryContextResetAndDeleteChildren.
We ask extension authors to deal with much more significant API changes
than that in every release, and versions where the updated code wouldn't
work are long gone. And, as you say, the existence of that separate from
MemoryContextReset creates confusion, which has nonzero cost in itself.
regards, tom lane
On Tue, Nov 14, 2023 at 9:50 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2023-Nov-13, Nathan Bossart wrote:
Shall we retire this backwards compatibility macro at this point? A search
of https://codesearch.debian.net/ does reveal a few external uses, so we
could alternatively leave it around and just update Postgres to stop using
it, but I don't think it would be too burdensome for extension authors to
fix if we removed it completely.Let's leave the macro around and just remove its uses in PGDG-owned
code. Having the macro around hurts nothing, and we can remove it in 15
years or so.
FWIW, there are other backward compatibility macros out there like
tuplestore_donestoring which was introduced by commit dd04e95 21 years
ago and SPI_push() and its friends which were made no-ops macros by
commit 1833f1a 7 years ago. Debian code search shows very minimal
usages of the above macros. Can we do away with
tuplestore_donestoring?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Tue, Nov 14, 2023 at 12:10:41PM -0500, Tom Lane wrote:
FWIW, I think it's fine to just nuke MemoryContextResetAndDeleteChildren.
We ask extension authors to deal with much more significant API changes
than that in every release, and versions where the updated code wouldn't
work are long gone. And, as you say, the existence of that separate from
MemoryContextReset creates confusion, which has nonzero cost in itself.
That is my preference as well. Alvaro, AFAICT you are the only vote
against removing it completely. If you feel ѕtrongly about it, I don't
mind going the __attribute__((deprecated)) route, but otherwise, I'd
probably just remove it completely.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, Nov 14, 2023 at 10:46:25PM +0530, Bharath Rupireddy wrote:
FWIW, there are other backward compatibility macros out there like
tuplestore_donestoring which was introduced by commit dd04e95 21 years
ago and SPI_push() and its friends which were made no-ops macros by
commit 1833f1a 7 years ago. Debian code search shows very minimal
usages of the above macros. Can we do away with
tuplestore_donestoring?
Can we take these other things to a separate thread?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On 2023-Nov-14, Nathan Bossart wrote:
On Tue, Nov 14, 2023 at 12:10:41PM -0500, Tom Lane wrote:
FWIW, I think it's fine to just nuke MemoryContextResetAndDeleteChildren.
We ask extension authors to deal with much more significant API changes
than that in every release, and versions where the updated code wouldn't
work are long gone. And, as you say, the existence of that separate from
MemoryContextReset creates confusion, which has nonzero cost in itself.That is my preference as well. Alvaro, AFAICT you are the only vote
against removing it completely. If you feel ѕtrongly about it,
Oh, I don't. (But I wouldn't mind putting pg_attribute_deprecated to
good use elsewhere ... not that I have any specific examples handy.)
Your S key seems to be doing some funny business.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Hay dos momentos en la vida de un hombre en los que no debería
especular: cuando puede permitírselo y cuando no puede" (Mark Twain)
On Tue, Nov 14, 2023 at 08:20:08PM +0100, Alvaro Herrera wrote:
Oh, I don't. (But I wouldn't mind putting pg_attribute_deprecated to
good use elsewhere ... not that I have any specific examples handy.)
Agreed.
Your S key seems to be doing some funny business.
I seem to have accidentally enabled "digraph" in my .vimrc at some point...
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Tue, Nov 14, 2023 at 9:21 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:
On Tue, Nov 14, 2023 at 04:25:24PM +0530, Amul Sul wrote:
Changes looks pretty much straight forward, but patch failed to apply on
the
latest master head(b41b1a7f490) at me.
Thanks for taking a look. Would you mind sharing the error(s) you are
seeing? The patch applies fine on cfbot and my machine, and check-world
continues to pass.
Nevermind, I usually use git apply or git am, here are those errors:
PG/ - (master) $ git apply ~/Downloads/retire_compatibility_macro_v1.patch
error: patch failed: src/backend/access/brin/brin.c:297
error: src/backend/access/brin/brin.c: patch does not apply
error: patch failed: src/backend/access/gin/ginscan.c:251
error: src/backend/access/gin/ginscan.c: patch does not apply
error: patch failed: src/backend/access/transam/xact.c:1933
error: src/backend/access/transam/xact.c: patch does not apply
error: patch failed: src/backend/commands/analyze.c:583
error: src/backend/commands/analyze.c: patch does not apply
error: patch failed: src/backend/executor/nodeRecursiveunion.c:317
error: src/backend/executor/nodeRecursiveunion.c: patch does not apply
error: patch failed: src/backend/executor/nodeSetOp.c:631
error: src/backend/executor/nodeSetOp.c: patch does not apply
error: patch failed: src/backend/executor/nodeWindowAgg.c:216
error: src/backend/executor/nodeWindowAgg.c: patch does not apply
error: patch failed: src/backend/executor/spi.c:547
error: src/backend/executor/spi.c: patch does not apply
error: patch failed: src/backend/postmaster/autovacuum.c:555
error: src/backend/postmaster/autovacuum.c: patch does not apply
error: patch failed: src/backend/postmaster/bgwriter.c:182
error: src/backend/postmaster/bgwriter.c: patch does not apply
error: patch failed: src/backend/postmaster/checkpointer.c:290
error: src/backend/postmaster/checkpointer.c: patch does not apply
error: patch failed: src/backend/postmaster/walwriter.c:178
error: src/backend/postmaster/walwriter.c: patch does not apply
error: patch failed: src/backend/replication/logical/worker.c:3647
error: src/backend/replication/logical/worker.c: patch does not apply
error: patch failed: src/backend/statistics/extended_stats.c:2237
error: src/backend/statistics/extended_stats.c: patch does not apply
error: patch failed: src/backend/tcop/postgres.c:4457
error: src/backend/tcop/postgres.c: patch does not apply
error: patch failed: src/backend/utils/cache/evtcache.c:91
error: src/backend/utils/cache/evtcache.c: patch does not apply
error: patch failed: src/backend/utils/error/elog.c:1833
error: src/backend/utils/error/elog.c: patch does not apply
error: patch failed: src/include/utils/memutils.h:66
error: src/include/utils/memutils.h: patch does not apply
PG/ - (master) $
Regards,
Amul
On Wed, Nov 15, 2023 at 09:27:18AM +0530, Amul Sul wrote:
Nevermind, I usually use git apply or git am, here are those errors:
PG/ - (master) $ git apply ~/Downloads/retire_compatibility_macro_v1.patch
error: patch failed: src/backend/access/brin/brin.c:297
error: src/backend/access/brin/brin.c: patch does not apply
I wonder if your mail client is modifying the patch. Do you have the same
issue if you download it from the archives?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Committed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
On Wed, Nov 15, 2023 at 9:26 PM Nathan Bossart <nathandbossart@gmail.com>
wrote:
On Wed, Nov 15, 2023 at 09:27:18AM +0530, Amul Sul wrote:
Nevermind, I usually use git apply or git am, here are those errors:
PG/ - (master) $ git apply
~/Downloads/retire_compatibility_macro_v1.patch
error: patch failed: src/backend/access/brin/brin.c:297
error: src/backend/access/brin/brin.c: patch does not applyI wonder if your mail client is modifying the patch. Do you have the same
issue if you download it from the archives?
Yes, you are correct. Surprisingly, the archive version applied cleanly.
Gmail is doing something, I usually use web login on chrome browser, I
never
faced such issues with other's patches. Anyway, will try both the versions
next
time for the same kind of issue, sorry for the noise.
Regards,
Amul
On Tue, Nov 14, 2023 at 11:29 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:
On Tue, Nov 14, 2023 at 10:46:25PM +0530, Bharath Rupireddy wrote:
FWIW, there are other backward compatibility macros out there like
tuplestore_donestoring which was introduced by commit dd04e95 21 years
ago and SPI_push() and its friends which were made no-ops macros by
commit 1833f1a 7 years ago. Debian code search shows very minimal
usages of the above macros. Can we do away with
tuplestore_donestoring?Can we take these other things to a separate thread?
Sure. Here it is -
/messages/by-id/CALj2ACVeO58JM5tK2Qa8QC-=kC8sdkJOTd4BFU=K8zs4gGYpjQ@mail.gmail.com.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com