agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedrefactoring fork() and EXEC_BACKEND
6+ messages / 4 participants
[nested] [flat]
* refactoring fork() and EXEC_BACKEND
@ 2005-03-04 05:30 Neil Conway <neilc@samurai.com>
0 siblings, 0 replies; 6+ messages in thread
From: Neil Conway @ 2005-03-04 05:30 UTC (permalink / raw)
To: pgsql-hackers
While going through the usual motions needed to fork a child process of
the postmaster, it occurred to me that there's a fair bit of duplicated
code involved. There are also #ifdef for various situations (BeOS,
LINUX_PROFILE, and EXEC_BACKEND), which makes the code yet more ugly. I
think we could make this a lot cleaner.
I'd like to define an API like so:
pid_t fork_process(int proc_type);
pid_t fork_backend(Port *port);
If the process needs to add a lot of private information to the argv in
the case of EXEC_BACKEND, they could invoke a third variant:
#ifdef EXEC_BACKEND
pid_t forkexec_process(int proc_type, int argc, char **argv);
#endif
(Or possibly using varargs, if that is cleaner for most call-sites).
Hopefully most call sites could just use fork_process().
These functions would then take care of all the necessary
platform-specific judo:
- flush stdout, stderr
- invoke BeOS hooks as necessary
- save and restore profiling timer, if necessary
- if EXEC_BACKEND, use proc_type to lay out the argv for the new process
and then invoke internal_forkexec()
- otherwise, just invoke fork()
- return result to client
So, most call sites would be quite nice:
pid_t result = fork_process(PROC_TYPE_FOO);
if (result == -1) { /* fork failed, in parent */ }
else if (result == 0) { /* in child */ }
else { /* in parent, `result' is pid of child */ }
I'd also like to move the implementation of fork_process() and friends,
as well as internal_forkexec(), into a separate file -- I'd rather not
clutter up postmaster.c with it.
Comments?
-Neil
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: refactoring fork() and EXEC_BACKEND
@ 2005-03-04 13:46 Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 6+ messages in thread
From: Magnus Hagander @ 2005-03-04 13:46 UTC (permalink / raw)
To: Neil Conway <neilc@samurai.com>; pgsql-hackers
>While going through the usual motions needed to fork a child
>process of
>the postmaster, it occurred to me that there's a fair bit of
>duplicated
>code involved. There are also #ifdef for various situations (BeOS,
>LINUX_PROFILE, and EXEC_BACKEND), which makes the code yet
>more ugly. I
>think we could make this a lot cleaner.
>
>I'd like to define an API like so:
This is a lot like what I was planning to work towards with the
refactoring of the forkexec code I promised to do for 8.1. Glad to hear
you think in the same direction.
>pid_t fork_process(int proc_type);
>pid_t fork_backend(Port *port);
>
>If the process needs to add a lot of private information to
>the argv in
>the case of EXEC_BACKEND, they could invoke a third variant:
>
>#ifdef EXEC_BACKEND
>pid_t forkexec_process(int proc_type, int argc, char **argv);
>#endif
>
>(Or possibly using varargs, if that is cleaner for most call-sites).
>Hopefully most call sites could just use fork_process().
I was actually thinking of not passing these on the commandline at all,
in order to avoid possible quoting issues etc (recall all the problems
with the stupid commandline processing on win32). Instead moving it into
a struct that is appended to the end of the backend variable file/shared
memory.
<snip>
>So, most call sites would be quite nice:
>
>pid_t result = fork_process(PROC_TYPE_FOO);
>if (result == -1) { /* fork failed, in parent */ }
>else if (result == 0) { /* in child */ }
>else { /* in parent, `result' is pid of child */ }
You're not going to be able to get the "in child" there for an execed
process, are you? it has to be called somewhere in the new process, and
thus it would have to be a function, wouldn't it?
>I'd also like to move the implementation of fork_process() and
>friends,
>as well as internal_forkexec(), into a separate file -- I'd rather not
>clutter up postmaster.c with it.
That was also what I was thinking. Let me know if you want to split the
load somewhere :-)
//Magnus
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: refactoring fork() and EXEC_BACKEND
@ 2005-03-05 08:59 Neil Conway <neilc@samurai.com>
parent: Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 6+ messages in thread
From: Neil Conway @ 2005-03-05 08:59 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: pgsql-hackers
Magnus Hagander wrote:
> This is a lot like what I was planning to work towards with the
> refactoring of the forkexec code I promised to do for 8.1.
Cool. BTW, have we accepted that EXEC_BACKEND is the way we're going to
workaround the lack of fork() on Win32 for the foreseeable future? I
mean, it _works_, but it's slow, ugly, and complicates the code. If it's
the only workable option for Win32 support, then fair enough -- I just
don't know enough of the Win32 API to know if there's a better
alternative out there (short of using threads, which is of course not
really plausible).
> I was actually thinking of not passing these on the commandline at all,
> in order to avoid possible quoting issues etc (recall all the problems
> with the stupid commandline processing on win32). Instead moving it into
> a struct that is appended to the end of the backend variable file/shared
> memory.
Sounds good to me. Finding a cleaner way to pass data to the child
process than writing it out to a file would also be nice, if possible.
Again, I'm not sure what options there are on Win32...
> That was also what I was thinking. Let me know if you want to split the
> load somewhere :-)
Given that you're planning to work on this, I've scaled back my
ambitions. I'll send a patch to -patches that just cleans up fork() and
doesn't change the EXEC_BACKEND case. So fork_process() will:
- flush stderr/stdout
- save and restore the profiling timer if LINUX_PROFILE is defined
- handle BeOS
which means it should not be very invasive. Of course, there is plenty
of room for improvement -- if you're interested in taking a look, please
do...
-Neil
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: refactoring fork() and EXEC_BACKEND
@ 2005-03-06 18:44 Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 6+ messages in thread
From: Magnus Hagander @ 2005-03-06 18:44 UTC (permalink / raw)
To: Neil Conway <neilc@samurai.com>; +Cc: pgsql-hackers
>> This is a lot like what I was planning to work towards with the
>> refactoring of the forkexec code I promised to do for 8.1.
>
>Cool. BTW, have we accepted that EXEC_BACKEND is the way we're
>going to
>workaround the lack of fork() on Win32 for the foreseeable future? I
>mean, it _works_, but it's slow, ugly, and complicates the
>code. If it's
>the only workable option for Win32 support, then fair enough -- I just
>don't know enough of the Win32 API to know if there's a better
>alternative out there (short of using threads, which is of course not
>really plausible).
I don't beleive there is any other way than using threads. The only
"objects" you can create are processes and threads, and I don't know
there to be any other way to create a process than CreateProcess().
>> I was actually thinking of not passing these on the
>commandline at all,
>> in order to avoid possible quoting issues etc (recall all
>the problems
>> with the stupid commandline processing on win32). Instead
>moving it into
>> a struct that is appended to the end of the backend variable
>file/shared
>> memory.
>
>Sounds good to me. Finding a cleaner way to pass data to the child
>process than writing it out to a file would also be nice, if possible.
>Again, I'm not sure what options there are on Win32...
Win32 already passes it using shared memory. It was when I asked to get
that patch in during beta (or possibly even RC) that I promised to work
on the cleanup stuff for 8.1. For unix/exec_backend it still writes to a
file, but since that is never expected to be used in production where
performance is an issue...
I think that is a fairly clean way of doing it. You could pass it
through a pipe or something, but I don't see that it would be a cleaner
approach. You're still going to have a single place collecting all the
data, which is where most of the uglyness comes from.
>> That was also what I was thinking. Let me know if you want
>to split the
>> load somewhere :-)
>
>Given that you're planning to work on this, I've scaled back my
>ambitions. I'll send a patch to -patches that just cleans up
>fork() and
>doesn't change the EXEC_BACKEND case. So fork_process() will:
>
>- flush stderr/stdout
>- save and restore the profiling timer if LINUX_PROFILE is defined
>- handle BeOS
>
>which means it should not be very invasive. Of course, there is plenty
>of room for improvement -- if you're interested in taking a
>look, please
>do...
Ok. I'll look at it once your stuff is done.
//Magnus
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v1 1/2] Avoid some calls to memset..
@ 2019-12-27 23:30 Justin Pryzby <pryzbyj@telsasoft.com>
0 siblings, 0 replies; 6+ messages in thread
From: Justin Pryzby @ 2019-12-27 23:30 UTC (permalink / raw)
..in cases where that saves a couple lines of code.
Note that gcc has builtin for memset, but inlined function is still not same as
initializing to zero.
That should probably be faster, since the local storage can be zerod by the
compiler during stack manipulation, and could possibly allow for
additional optimization, too.
---
contrib/pageinspect/ginfuncs.c | 12 +++---------
contrib/pageinspect/heapfuncs.c | 4 +---
contrib/pageinspect/rawpage.c | 4 +---
contrib/pgstattuple/pgstatapprox.c | 4 +---
src/backend/catalog/pg_collation.c | 4 +---
src/backend/catalog/pg_db_role_setting.c | 4 +---
src/backend/catalog/pg_depend.c | 4 +---
src/backend/catalog/pg_enum.c | 6 ++----
src/backend/catalog/pg_inherits.c | 4 +---
src/backend/catalog/pg_range.c | 4 +---
src/backend/catalog/pg_shdepend.c | 8 ++------
src/backend/commands/event_trigger.c | 3 +--
src/backend/commands/indexcmds.c | 3 +--
src/backend/commands/seclabel.c | 12 ++++--------
src/backend/commands/sequence.c | 12 +++---------
src/backend/commands/trigger.c | 4 +---
src/backend/commands/tsearchcmds.c | 3 +--
src/backend/replication/logical/logicalfuncs.c | 3 +--
src/backend/replication/slotfuncs.c | 8 ++------
src/backend/replication/walsender.c | 3 +--
src/backend/statistics/mcv.c | 5 +----
src/backend/utils/adt/genfile.c | 4 +---
22 files changed, 32 insertions(+), 86 deletions(-)
diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c
index 4b623fb..d9590bd 100644
--- a/contrib/pageinspect/ginfuncs.c
+++ b/contrib/pageinspect/ginfuncs.c
@@ -40,7 +40,7 @@ gin_metapage_info(PG_FUNCTION_ARGS)
GinMetaPageData *metadata;
HeapTuple resultTuple;
Datum values[10];
- bool nulls[10];
+ bool nulls[10] = {0,};
if (!superuser())
ereport(ERROR,
@@ -63,8 +63,6 @@ gin_metapage_info(PG_FUNCTION_ARGS)
metadata = GinPageGetMeta(page);
- memset(nulls, 0, sizeof(nulls));
-
values[0] = Int64GetDatum(metadata->head);
values[1] = Int64GetDatum(metadata->tail);
values[2] = Int32GetDatum(metadata->tailFreeSize);
@@ -95,7 +93,7 @@ gin_page_opaque_info(PG_FUNCTION_ARGS)
GinPageOpaque opaq;
HeapTuple resultTuple;
Datum values[3];
- bool nulls[3];
+ bool nulls[3] = {0,};
Datum flags[16];
int nflags = 0;
uint16 flagbits;
@@ -139,8 +137,6 @@ gin_page_opaque_info(PG_FUNCTION_ARGS)
flags[nflags++] = DirectFunctionCall1(to_hex32, Int32GetDatum(flagbits));
}
- memset(nulls, 0, sizeof(nulls));
-
values[0] = Int64GetDatum(opaq->rightlink);
values[1] = Int32GetDatum(opaq->maxoff);
values[2] = PointerGetDatum(construct_array(flags, nflags,
@@ -227,14 +223,12 @@ gin_leafpage_items(PG_FUNCTION_ARGS)
HeapTuple resultTuple;
Datum result;
Datum values[3];
- bool nulls[3];
+ bool nulls[3] = {0,};
int ndecoded,
i;
ItemPointer tids;
Datum *tids_datum;
- memset(nulls, 0, sizeof(nulls));
-
values[0] = ItemPointerGetDatum(&cur->first);
values[1] = UInt16GetDatum(cur->nbytes);
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index aa7e4b9..5b8ddf3 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -179,13 +179,11 @@ heap_page_items(PG_FUNCTION_ARGS)
Datum result;
ItemId id;
Datum values[14];
- bool nulls[14];
+ bool nulls[14] = {0,};
uint16 lp_offset;
uint16 lp_flags;
uint16 lp_len;
- memset(nulls, 0, sizeof(nulls));
-
/* Extract information from the line pointer */
id = PageGetItemId(page, inter_call_data->offset);
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index a7b0d17..0901429 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -225,7 +225,7 @@ page_header(PG_FUNCTION_ARGS)
Datum result;
HeapTuple tuple;
Datum values[9];
- bool nulls[9];
+ bool nulls[9] = {0,};
PageHeader page;
XLogRecPtr lsn;
@@ -278,8 +278,6 @@ page_header(PG_FUNCTION_ARGS)
/* Build and return the tuple. */
- memset(nulls, 0, sizeof(nulls));
-
tuple = heap_form_tuple(tupdesc, values, nulls);
result = HeapTupleGetDatum(tuple);
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 672dbf2..826db15 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -254,7 +254,7 @@ pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo)
Relation rel;
output_type stat = {0};
TupleDesc tupdesc;
- bool nulls[NUM_OUTPUT_COLUMNS];
+ bool nulls[NUM_OUTPUT_COLUMNS] = {0,};
Datum values[NUM_OUTPUT_COLUMNS];
HeapTuple ret;
int i = 0;
@@ -297,8 +297,6 @@ pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo)
relation_close(rel, AccessShareLock);
- memset(nulls, 0, sizeof(nulls));
-
values[i++] = Int64GetDatum(stat.table_len);
values[i++] = Float8GetDatum(stat.scanned_percent);
values[i++] = Int64GetDatum(stat.tuple_count);
diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c
index 8559779..f818857 100644
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -57,7 +57,7 @@ CollationCreate(const char *collname, Oid collnamespace,
TupleDesc tupDesc;
HeapTuple tup;
Datum values[Natts_pg_collation];
- bool nulls[Natts_pg_collation];
+ bool nulls[Natts_pg_collation] = {0,};
NameData name_name,
name_collate,
name_ctype;
@@ -151,8 +151,6 @@ CollationCreate(const char *collname, Oid collnamespace,
tupDesc = RelationGetDescr(rel);
/* form a tuple */
- memset(nulls, 0, sizeof(nulls));
-
namestrcpy(&name_name, collname);
oid = GetNewOidWithIndex(rel, CollationOidIndexId,
Anum_pg_collation_oid);
diff --git a/src/backend/catalog/pg_db_role_setting.c b/src/backend/catalog/pg_db_role_setting.c
index 33fc53a..58a32a4 100644
--- a/src/backend/catalog/pg_db_role_setting.c
+++ b/src/backend/catalog/pg_db_role_setting.c
@@ -136,11 +136,9 @@ AlterSetting(Oid databaseid, Oid roleid, VariableSetStmt *setstmt)
/* non-null valuestr means it's not RESET, so insert a new tuple */
HeapTuple newtuple;
Datum values[Natts_pg_db_role_setting];
- bool nulls[Natts_pg_db_role_setting];
+ bool nulls[Natts_pg_db_role_setting] = {0,};
ArrayType *a;
- memset(nulls, false, sizeof(nulls));
-
a = GUCArrayAdd(NULL, setstmt->name, valuestr);
values[Anum_pg_db_role_setting_setdatabase - 1] =
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index f9af245..832ff76 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -61,7 +61,7 @@ recordMultipleDependencies(const ObjectAddress *depender,
CatalogIndexState indstate;
HeapTuple tup;
int i;
- bool nulls[Natts_pg_depend];
+ bool nulls[Natts_pg_depend] = {0,};
Datum values[Natts_pg_depend];
if (nreferenced <= 0)
@@ -79,8 +79,6 @@ recordMultipleDependencies(const ObjectAddress *depender,
/* Don't open indexes unless we need to make an update */
indstate = NULL;
- memset(nulls, false, sizeof(nulls));
-
for (i = 0; i < nreferenced; i++, referenced++)
{
/*
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index 27e4100..0146955 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -65,7 +65,7 @@ EnumValuesCreate(Oid enumTypeOid, List *vals)
int elemno,
num_elems;
Datum values[Natts_pg_enum];
- bool nulls[Natts_pg_enum];
+ bool nulls[Natts_pg_enum] = {0,};
ListCell *lc;
HeapTuple tup;
@@ -110,7 +110,6 @@ EnumValuesCreate(Oid enumTypeOid, List *vals)
qsort(oids, num_elems, sizeof(Oid), oid_cmp);
/* and make the entries */
- memset(nulls, false, sizeof(nulls));
elemno = 0;
foreach(lc, vals)
@@ -214,7 +213,7 @@ AddEnumLabel(Oid enumTypeOid,
Relation pg_enum;
Oid newOid;
Datum values[Natts_pg_enum];
- bool nulls[Natts_pg_enum];
+ bool nulls[Natts_pg_enum] = {0,};
NameData enumlabel;
HeapTuple enum_tup;
float4 newelemorder;
@@ -479,7 +478,6 @@ restart:
ReleaseCatCacheList(list);
/* Create the new pg_enum entry */
- memset(nulls, false, sizeof(nulls));
values[Anum_pg_enum_oid - 1] = ObjectIdGetDatum(newOid);
values[Anum_pg_enum_enumtypid - 1] = ObjectIdGetDatum(enumTypeOid);
values[Anum_pg_enum_enumsortorder - 1] = Float4GetDatum(newelemorder);
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index 17f37eb..3a98fa1 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -417,7 +417,7 @@ void
StoreSingleInheritance(Oid relationId, Oid parentOid, int32 seqNumber)
{
Datum values[Natts_pg_inherits];
- bool nulls[Natts_pg_inherits];
+ bool nulls[Natts_pg_inherits] = {0,};
HeapTuple tuple;
Relation inhRelation;
@@ -430,8 +430,6 @@ StoreSingleInheritance(Oid relationId, Oid parentOid, int32 seqNumber)
values[Anum_pg_inherits_inhparent - 1] = ObjectIdGetDatum(parentOid);
values[Anum_pg_inherits_inhseqno - 1] = Int32GetDatum(seqNumber);
- memset(nulls, 0, sizeof(nulls));
-
tuple = heap_form_tuple(RelationGetDescr(inhRelation), values, nulls);
CatalogTupleInsert(inhRelation, tuple);
diff --git a/src/backend/catalog/pg_range.c b/src/backend/catalog/pg_range.c
index b5bc36c..36a2103 100644
--- a/src/backend/catalog/pg_range.c
+++ b/src/backend/catalog/pg_range.c
@@ -39,15 +39,13 @@ RangeCreate(Oid rangeTypeOid, Oid rangeSubType, Oid rangeCollation,
{
Relation pg_range;
Datum values[Natts_pg_range];
- bool nulls[Natts_pg_range];
+ bool nulls[Natts_pg_range] = {0,};
HeapTuple tup;
ObjectAddress myself;
ObjectAddress referenced;
pg_range = table_open(RangeRelationId, RowExclusiveLock);
- memset(nulls, 0, sizeof(nulls));
-
values[Anum_pg_range_rngtypid - 1] = ObjectIdGetDatum(rangeTypeOid);
values[Anum_pg_range_rngsubtype - 1] = ObjectIdGetDatum(rangeSubType);
values[Anum_pg_range_rngcollation - 1] = ObjectIdGetDatum(rangeCollation);
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 2ef792d..ff05868 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -272,9 +272,7 @@ shdepChangeDep(Relation sdepRel,
{
/* Need to insert new entry */
Datum values[Natts_pg_shdepend];
- bool nulls[Natts_pg_shdepend];
-
- memset(nulls, false, sizeof(nulls));
+ bool nulls[Natts_pg_shdepend] = {0,};
values[Anum_pg_shdepend_dbid - 1] = ObjectIdGetDatum(dbid);
values[Anum_pg_shdepend_classid - 1] = ObjectIdGetDatum(classid);
@@ -933,7 +931,7 @@ shdepAddDependency(Relation sdepRel,
{
HeapTuple tup;
Datum values[Natts_pg_shdepend];
- bool nulls[Natts_pg_shdepend];
+ bool nulls[Natts_pg_shdepend] = {0,};
/*
* Make sure the object doesn't go away while we record the dependency on
@@ -942,8 +940,6 @@ shdepAddDependency(Relation sdepRel,
*/
shdepLockAndCheckObject(refclassId, refobjId);
- memset(nulls, false, sizeof(nulls));
-
/*
* Form the new tuple and record the dependency.
*/
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 6d4154a..864cee2 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -380,7 +380,7 @@ insert_event_trigger_tuple(const char *trigname, const char *eventname, Oid evtO
Oid trigoid;
HeapTuple tuple;
Datum values[Natts_pg_trigger];
- bool nulls[Natts_pg_trigger];
+ bool nulls[Natts_pg_trigger] = {0,};
NameData evtnamedata,
evteventdata;
ObjectAddress myself,
@@ -393,7 +393,6 @@ insert_event_trigger_tuple(const char *trigname, const char *eventname, Oid evtO
trigoid = GetNewOidWithIndex(tgrel, EventTriggerOidIndexId,
Anum_pg_event_trigger_oid);
values[Anum_pg_event_trigger_oid - 1] = ObjectIdGetDatum(trigoid);
- memset(nulls, false, sizeof(nulls));
namestrcpy(&evtnamedata, trigname);
values[Anum_pg_event_trigger_evtname - 1] = NameGetDatum(&evtnamedata);
namestrcpy(&evteventdata, eventname);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7b33b98..f93dc7b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3400,7 +3400,7 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
else
{
Datum values[Natts_pg_inherits];
- bool isnull[Natts_pg_inherits];
+ bool isnull[Natts_pg_inherits] = {0,};
/*
* No pg_inherits row exists, and we want a parent for this index,
@@ -3410,7 +3410,6 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
values[Anum_pg_inherits_inhparent - 1] =
ObjectIdGetDatum(parentOid);
values[Anum_pg_inherits_inhseqno - 1] = Int32GetDatum(1);
- memset(isnull, false, sizeof(isnull));
tuple = heap_form_tuple(RelationGetDescr(pg_inherits),
values, isnull);
diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c
index b497c06..e64fe96 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -258,12 +258,10 @@ SetSharedSecurityLabel(const ObjectAddress *object,
HeapTuple oldtup;
HeapTuple newtup = NULL;
Datum values[Natts_pg_shseclabel];
- bool nulls[Natts_pg_shseclabel];
- bool replaces[Natts_pg_shseclabel];
+ bool nulls[Natts_pg_shseclabel] = {0,};
+ bool replaces[Natts_pg_shseclabel] = {0,};
/* Prepare to form or update a tuple, if necessary. */
- memset(nulls, false, sizeof(nulls));
- memset(replaces, false, sizeof(replaces));
values[Anum_pg_shseclabel_objoid - 1] = ObjectIdGetDatum(object->objectId);
values[Anum_pg_shseclabel_classoid - 1] = ObjectIdGetDatum(object->classId);
values[Anum_pg_shseclabel_provider - 1] = CStringGetTextDatum(provider);
@@ -333,8 +331,8 @@ SetSecurityLabel(const ObjectAddress *object,
HeapTuple oldtup;
HeapTuple newtup = NULL;
Datum values[Natts_pg_seclabel];
- bool nulls[Natts_pg_seclabel];
- bool replaces[Natts_pg_seclabel];
+ bool nulls[Natts_pg_seclabel] = {0,};
+ bool replaces[Natts_pg_seclabel] = {0,};
/* Shared objects have their own security label catalog. */
if (IsSharedRelation(object->classId))
@@ -344,8 +342,6 @@ SetSecurityLabel(const ObjectAddress *object,
}
/* Prepare to form or update a tuple, if necessary. */
- memset(nulls, false, sizeof(nulls));
- memset(replaces, false, sizeof(replaces));
values[Anum_pg_seclabel_objoid - 1] = ObjectIdGetDatum(object->objectId);
values[Anum_pg_seclabel_classoid - 1] = ObjectIdGetDatum(object->classId);
values[Anum_pg_seclabel_objsubid - 1] = Int32GetDatum(object->objectSubId);
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 6aab73b..f7ea908 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -128,9 +128,9 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
HeapTuple tuple;
TupleDesc tupDesc;
Datum value[SEQ_COL_LASTCOL];
- bool null[SEQ_COL_LASTCOL];
+ bool null[SEQ_COL_LASTCOL] = {0,};
Datum pgs_values[Natts_pg_sequence];
- bool pgs_nulls[Natts_pg_sequence];
+ bool pgs_nulls[Natts_pg_sequence] = {0,};
int i;
/* Unlogged sequences are not implemented -- not clear if useful. */
@@ -182,8 +182,6 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
coldef->constraints = NIL;
coldef->location = -1;
- null[i - 1] = false;
-
switch (i)
{
case SEQ_COL_LASTVAL:
@@ -234,8 +232,6 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
rel = table_open(SequenceRelationId, RowExclusiveLock);
tupDesc = RelationGetDescr(rel);
- memset(pgs_nulls, 0, sizeof(pgs_nulls));
-
pgs_values[Anum_pg_sequence_seqrelid - 1] = ObjectIdGetDatum(seqoid);
pgs_values[Anum_pg_sequence_seqtypid - 1] = ObjectIdGetDatum(seqform.seqtypid);
pgs_values[Anum_pg_sequence_seqstart - 1] = Int64GetDatumFast(seqform.seqstart);
@@ -1790,7 +1786,7 @@ pg_sequence_parameters(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
TupleDesc tupdesc;
Datum values[7];
- bool isnull[7];
+ bool isnull[7] = {0,};
HeapTuple pgstuple;
Form_pg_sequence pgsform;
@@ -1818,8 +1814,6 @@ pg_sequence_parameters(PG_FUNCTION_ARGS)
BlessTupleDesc(tupdesc);
- memset(isnull, 0, sizeof(isnull));
-
pgstuple = SearchSysCache1(SEQRELID, relid);
if (!HeapTupleIsValid(pgstuple))
elog(ERROR, "cache lookup failed for sequence %u", relid);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index c5c7b21..e7da3f3 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -171,7 +171,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
List *whenRtable;
char *qual;
Datum values[Natts_pg_trigger];
- bool nulls[Natts_pg_trigger];
+ bool nulls[Natts_pg_trigger] = {0,};
Relation rel;
AclResult aclresult;
Relation tgrel;
@@ -843,8 +843,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
* makes the triggers in partitions identical to the ones in the
* partitioned tables, except that they are marked internal.
*/
- memset(nulls, false, sizeof(nulls));
-
values[Anum_pg_trigger_oid - 1] = ObjectIdGetDatum(trigoid);
values[Anum_pg_trigger_tgrelid - 1] = ObjectIdGetDatum(RelationGetRelid(rel));
values[Anum_pg_trigger_tgname - 1] = DirectFunctionCall1(namein,
diff --git a/src/backend/commands/tsearchcmds.c b/src/backend/commands/tsearchcmds.c
index 9dca682..65e4cf8 100644
--- a/src/backend/commands/tsearchcmds.c
+++ b/src/backend/commands/tsearchcmds.c
@@ -1420,9 +1420,8 @@ MakeConfigurationMapping(AlterTSConfigurationStmt *stmt,
for (j = 0; j < ndict; j++)
{
Datum values[Natts_pg_ts_config_map];
- bool nulls[Natts_pg_ts_config_map];
+ bool nulls[Natts_pg_ts_config_map] = {0,};
- memset(nulls, false, sizeof(nulls));
values[Anum_pg_ts_config_map_mapcfg - 1] = ObjectIdGetDatum(cfgId);
values[Anum_pg_ts_config_map_maptokentype - 1] = Int32GetDatum(tokens[i]);
values[Anum_pg_ts_config_map_mapseqno - 1] = Int32GetDatum(j + 1);
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 7693c98..cb23dd6 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -67,7 +67,7 @@ LogicalOutputWrite(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xi
bool last_write)
{
Datum values[3];
- bool nulls[3];
+ bool nulls[3] = {0,};
DecodingOutputState *p;
/* SQL Datums can only be of a limited length... */
@@ -76,7 +76,6 @@ LogicalOutputWrite(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xi
p = (DecodingOutputState *) ctx->output_writer_private;
- memset(nulls, 0, sizeof(nulls));
values[0] = LSNGetDatum(lsn);
values[1] = TransactionIdGetDatum(xid);
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index bb69683..6cb4c6d 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -167,7 +167,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
TupleDesc tupdesc;
HeapTuple tuple;
Datum values[2];
- bool nulls[2];
+ bool nulls[2] = {0,};
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
@@ -184,8 +184,6 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
values[0] = NameGetDatum(&MyReplicationSlot->data.name);
values[1] = LSNGetDatum(MyReplicationSlot->data.confirmed_flush);
- memset(nulls, 0, sizeof(nulls));
-
tuple = heap_form_tuple(tupdesc, values, nulls);
result = HeapTupleGetDatum(tuple);
@@ -264,7 +262,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
{
ReplicationSlot *slot = &ReplicationSlotCtl->replication_slots[slotno];
Datum values[PG_GET_REPLICATION_SLOTS_COLS];
- bool nulls[PG_GET_REPLICATION_SLOTS_COLS];
+ bool nulls[PG_GET_REPLICATION_SLOTS_COLS] = {0,};
ReplicationSlotPersistency persistency;
TransactionId xmin;
@@ -294,8 +292,6 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
SpinLockRelease(&slot->mutex);
- memset(nulls, 0, sizeof(nulls));
-
i = 0;
values[i++] = NameGetDatum(&slot_name);
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 6e80e67..03c1e28 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3237,7 +3237,7 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
int64 spillCount;
int64 spillBytes;
Datum values[PG_STAT_GET_WAL_SENDERS_COLS];
- bool nulls[PG_STAT_GET_WAL_SENDERS_COLS];
+ bool nulls[PG_STAT_GET_WAL_SENDERS_COLS] = {0,};
SpinLockAcquire(&walsnd->mutex);
if (walsnd->pid == 0)
@@ -3261,7 +3261,6 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS)
spillBytes = walsnd->spillBytes;
SpinLockRelease(&walsnd->mutex);
- memset(nulls, 0, sizeof(nulls));
values[0] = Int32GetDatum(pid);
if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS))
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 87e232f..199453a 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1380,7 +1380,7 @@ pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
if (funcctx->call_cntr < funcctx->max_calls) /* do when there is more left to send */
{
Datum values[5];
- bool nulls[5];
+ bool nulls[5] = {0,};
HeapTuple tuple;
Datum result;
ArrayBuildState *astate_values = NULL;
@@ -1440,9 +1440,6 @@ pg_stats_ext_mcvlist_items(PG_FUNCTION_ARGS)
values[3] = Float8GetDatum(item->frequency);
values[4] = Float8GetDatum(item->base_frequency);
- /* no NULLs in the tuple */
- memset(nulls, 0, sizeof(nulls));
-
/* build a tuple */
tuple = heap_form_tuple(funcctx->attinmeta->tupdesc, values, nulls);
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 81b3d82..f7882c7 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -365,7 +365,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
char *filename;
struct stat fst;
Datum values[6];
- bool isnull[6];
+ bool isnull[6] = {0,};
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
@@ -405,8 +405,6 @@ pg_stat_file(PG_FUNCTION_ARGS)
"isdir", BOOLOID, -1, 0);
BlessTupleDesc(tupdesc);
- memset(isnull, false, sizeof(isnull));
-
values[0] = Int64GetDatum((int64) fst.st_size);
values[1] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_atime));
values[2] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_mtime));
--
2.7.4
--+QahgC5+KEYLbs62
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0002-Some-less-useful-changes-to-avoid-memset.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH] Add hook for plugins to acquire sample rows during ANALYZE
@ 2026-05-29 12:35 Samba Siva <sambasivareddy.ch@zohomail.in>
0 siblings, 0 replies; 6+ messages in thread
From: Samba Siva @ 2026-05-29 12:35 UTC (permalink / raw)
- Introduced AcquireSampleRowsFunc_hook for extensions to override row sampling.
- Updated analyze.c to utilize the hook if registered.
- Added tests to ensure ANALYZE completes without errors with the new hook.
---
doc/src/sgml/xfunc.sgml | 10 ++++++++++
src/backend/commands/analyze.c | 24 ++++++++++++++++++++----
src/include/commands/vacuum.h | 11 +++++++++++
src/test/regress/expected/analyze.out | 20 ++++++++++++++++++++
src/test/regress/parallel_schedule | 1 +
src/test/regress/sql/analyze.sql | 24 ++++++++++++++++++++++++
6 files changed, 86 insertions(+), 4 deletions(-)
create mode 100644 src/test/regress/expected/analyze.out
create mode 100644 src/test/regress/sql/analyze.sql
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 1eb5abffd8..f1ef025ae2 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -4203,4 +4203,14 @@ supportfn(internal) returns internal
To create such conditions, the support function must implement
the <literal>SupportRequestIndexCondition</literal> request type.
</para>
+
+ <para>
+ The hook variable <varname>AcquireSampleRowsFunc_hook</varname> allows extensions
+ to override the row sampling function during <command>ANALYZE</command> for regular
+ heap relations. This is useful for extensions/systems which implement distributed
+ databases and want to sample rows from remote nodes instead of the local heap.
+ The hook function fills the provided <literal>rows[]</literal> buffer with at most
+ <literal>targrows</literal> heap tuples and sets <literal>*totalrows</literal> to the
+ estimated total live row count of the relation.
+ </para>
</sect1>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 4fffb76e55..3560acdff1 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -74,6 +74,8 @@ int default_statistics_target = 100;
static MemoryContext anl_context = NULL;
static BufferAccessStrategy vac_strategy;
+/* Hook for plugins to acquire sample rows for ANALYZE */
+AcquireSampleRowsFunc_hook_type AcquireSampleRowsFunc_hook = NULL;
static void do_analyze_rel(Relation onerel,
VacuumParams *params, List *va_cols,
@@ -188,8 +190,15 @@ analyze_rel(Oid relid, RangeVar *relation,
if (onerel->rd_rel->relkind == RELKIND_RELATION ||
onerel->rd_rel->relkind == RELKIND_MATVIEW)
{
- /* Regular table, so we'll use the regular row acquisition function */
- acquirefunc = acquire_sample_rows;
+ /*
+ * Regular table, so we'll use the regular row acquisition function.
+ * If a plugin has registered a hook to acquire sample rows, use it;
+ * otherwise use the default function.
+ */
+ if (AcquireSampleRowsFunc_hook)
+ acquirefunc = AcquireSampleRowsFunc_hook;
+ else
+ acquirefunc = acquire_sample_rows;
/* Also get regular table's size */
relpages = RelationGetNumberOfBlocks(onerel);
}
@@ -1467,8 +1476,15 @@ acquire_inherited_sample_rows(Relation onerel, int elevel,
if (childrel->rd_rel->relkind == RELKIND_RELATION ||
childrel->rd_rel->relkind == RELKIND_MATVIEW)
{
- /* Regular table, so use the regular row acquisition function */
- acquirefunc = acquire_sample_rows;
+ /*
+ * Regular table, so use the regular row acquisition function.
+ * If a plugin has registered a hook to acquire sample rows, use it;
+ * otherwise use the default function.
+ */
+ if (AcquireSampleRowsFunc_hook)
+ acquirefunc = AcquireSampleRowsFunc_hook;
+ else
+ acquirefunc = acquire_sample_rows;
relpages = RelationGetNumberOfBlocks(childrel);
}
else if (childrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index bc37a80dc7..146f936861 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -21,6 +21,7 @@
#include "catalog/pg_class.h"
#include "catalog/pg_statistic.h"
#include "catalog/pg_type.h"
+#include "foreign/fdwapi.h"
#include "parser/parse_node.h"
#include "storage/buf.h"
#include "storage/lock.h"
@@ -113,6 +114,9 @@ typedef void (*AnalyzeAttrComputeStatsFunc) (VacAttrStatsP stats,
int samplerows,
double totalrows);
+/* Hook type for plugins to acquire sample rows for ANALYZE */
+typedef AcquireSampleRowsFunc AcquireSampleRowsFunc_hook_type;
+
typedef struct VacAttrStats
{
/*
@@ -334,6 +338,13 @@ extern PGDLLIMPORT int vacuum_cost_limit;
extern PGDLLIMPORT int64 parallel_vacuum_worker_delay_ns;
+/*
+ * Hook for plugins to override row sampling during ANALYZE.
+ * Also applies to child relations of partitioned/inherited tables.
+ * See acquire_sample_rows() in src/backend/commands/analyze.c.
+ */
+extern PGDLLIMPORT AcquireSampleRowsFunc_hook_type AcquireSampleRowsFunc_hook;
+
/* in commands/vacuum.c */
extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel);
extern void vacuum(List *relations, VacuumParams *params,
diff --git a/src/test/regress/expected/analyze.out b/src/test/regress/expected/analyze.out
new file mode 100644
index 0000000000..5bb6bbc45b
--- /dev/null
+++ b/src/test/regress/expected/analyze.out
@@ -0,0 +1,20 @@
+-- Test AcquireSampleRowsFunc_hook
+-- Usually this would be tested via a C extension.
+-- Here we just confirm this does not break the existing ANALYZE code
+-- by verifying that ANALYZE completes without error.
+CREATE TABLE employees (
+ id SERIAL PRIMARY KEY,
+ name TEXT,
+ department TEXT,
+ salary NUMERIC
+);
+INSERT INTO employees
+ SELECT
+ i,
+ 'Employee ' || i,
+ 'Department ' || (i % 5),
+ (i % 100) * 1000 + 50000
+ FROM generate_series(1, 1000) i;
+-- Should complete without error
+ANALYZE employees;
+DROP TABLE employees;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index e1e0c54019..71978ef858 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -94,6 +94,7 @@ test: vacuum_parallel
# Run this alone, because concurrent DROP TABLE would make non-superuser
# "ANALYZE;" fail with "relation with OID $n does not exist".
test: maintain_every
+test: analyze
# no relation related tests can be put in this group
test: publication subscription
diff --git a/src/test/regress/sql/analyze.sql b/src/test/regress/sql/analyze.sql
new file mode 100644
index 0000000000..e0bc84e9f9
--- /dev/null
+++ b/src/test/regress/sql/analyze.sql
@@ -0,0 +1,24 @@
+-- Test AcquireSampleRowsFunc_hook
+-- Usually this would be tested via a C extension.
+-- Here we just confirm this does not break the existing ANALYZE code
+-- by verifying that ANALYZE completes without error.
+
+CREATE TABLE employees (
+ id SERIAL PRIMARY KEY,
+ name TEXT,
+ department TEXT,
+ salary NUMERIC
+);
+
+INSERT INTO employees
+ SELECT
+ i,
+ 'Employee ' || i,
+ 'Department ' || (i % 5),
+ (i % 100) * 1000 + 50000
+ FROM generate_series(1, 1000) i;
+
+-- Should complete without error
+ANALYZE employees;
+
+DROP TABLE employees;
--
2.50.1 (Apple Git-155)
------=_Part_741589_445982825.1782438414800--
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-05-29 12:35 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2005-03-04 05:30 refactoring fork() and EXEC_BACKEND Neil Conway <neilc@samurai.com>
2005-03-04 13:46 Re: refactoring fork() and EXEC_BACKEND Magnus Hagander <mha@sollentuna.net>
2005-03-05 08:59 ` Neil Conway <neilc@samurai.com>
2005-03-06 18:44 Re: refactoring fork() and EXEC_BACKEND Magnus Hagander <mha@sollentuna.net>
2019-12-27 23:30 [PATCH v1 1/2] Avoid some calls to memset.. Justin Pryzby <pryzbyj@telsasoft.com>
2026-05-29 12:35 [PATCH] Add hook for plugins to acquire sample rows during ANALYZE Samba Siva <sambasivareddy.ch@zohomail.in>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox